* RE: [PATCHv5 1/4] modem_shm: Add Modem Access Framework
From: Arun MURTHY @ 2012-10-17 8:00 UTC (permalink / raw)
To: Greg KH
Cc: linux-kernel@vger.kernel.org, netdev@vger.kernel.org,
linux-doc@vger.kernel.org, alan@lxorguk.ukuu.org.uk
In-Reply-To: <20121015212943.GA12540@kroah.com>
> On Mon, Oct 15, 2012 at 10:58:37AM +0530, Arun Murthy wrote:
>
> I'm going to ignore your .c logic, as there are things in it that I don't think is
> correct. But it all comes down to your data structures, if you fix them, then
> the .c logic will become correct:
>
> > --- /dev/null
> > +++ b/include/linux/modem_shm/modem.h
> > @@ -0,0 +1,59 @@
> > +/*
> > + * Copyright (C) ST-Ericsson SA 2011
> > + *
> > + * License Terms: GNU General Public License v2
> > + * Author: Kumar Sanghvi
> > + * Arun Murthy <arun.murthy@stericsson.com>
> > + *
> > + * Heavily adapted from Regulator framework */ #ifndef __MODEM_H__
> > +#define __MODEM_H__
>
> __MODEM_SHM_MODEM_H__, right?
>
Done!
> > +
> > +#include <linux/device.h>
> > +
> > +struct clients {
> > + struct device *dev;
>
> Why is this a pointer? It should be embedded in the structure.
This is the pointer to the clients's device structure. There will be a
single entity for a modem but multiple clients. Hence this struct is
specific for each client.
>
> > + const char *name;
>
> Why is this needed? It should be the same as the device, right?
Ok, will remove this.
>
> > + atomic_t cnt;
>
> Why is this needed at all? And if it's needed, why is it an atomic?
> (hint, your use of atomic_t really isn't correct at all in this patch, it's not doing
> what you think it is doing...)
Basically the operation done by this, i.e modem access/release has a lot of
affect on the clients. Since we are accesing some modem registers without
this modem request it might lead to system freeze. Hence such cross over
scenarios are considered and the counter is increased/decreased after the
operation(modem access/release). And reading of these variable are done
especially during system suspend, where decision is taken based on the value
of the counter read, hence any modification done should preside over read.
Moreover since there are multiple clients, using atomic for a simple locking
mechanism.
>
> > +};
>
> Also, the name of the structure here is _VERY_ generic, that's not acceptable
> in the global kernel namespace. Hint, it probably isn't even needed to be
> defined in this .h file at all, right?
Yes, even I felt so yest while reviewing, will change it accordingly.
>
> > +
> > +struct modem_desc {
> > + int (*request)(struct modem_desc *);
> > + void (*release)(struct modem_desc *);
> > + int (*is_requested)(struct modem_desc *);
> > + struct clients *mclients;
>
> Why do you have a pointer to a device, and yet:
This pointer to device is for the clients device.
>
> > + struct device *dev;
>
> have a device here?
This pointer to device is the modem's device struct.
>
> > + char *name;
>
> Same *dev and name comment as above.
Ok will remove this.
>
> > + u8 no_clients;
> > + atomic_t use_cnt;
> > + atomic_t cli_cnt;
>
> Same question about these atomic_t variables, why are they here, and most
> importantly, why are they an atomic variable?
Explained above.
Thanks and Regards,
Arun R Murthy
------------------
^ permalink raw reply
* [PATCH] ixgbe: convert to use simple_open()
From: Wei Yongjun @ 2012-10-17 8:26 UTC (permalink / raw)
To: jeffrey.t.kirsher, jesse.brandeburg, bruce.w.allan,
carolyn.wyborny, donald.c.skidmore, gregory.v.rose,
peter.p.waskiewicz.jr, alexander.h.duyck, john.ronciak
Cc: yongjun_wei, e1000-devel, netdev
From: Wei Yongjun <yongjun_wei@trendmicro.com.cn>
This removes an open coded simple_open() function and
replaces file operations references to the function
with simple_open() instead.
dpatch engine is used to auto generate this patch.
(https://github.com/weiyj/dpatch)
Signed-off-by: Wei Yongjun <yongjun_wei@trendmicro.com.cn>
---
drivers/net/ethernet/intel/ixgbe/ixgbe_debugfs.c | 32 ++----------------------
1 file changed, 2 insertions(+), 30 deletions(-)
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_debugfs.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_debugfs.c
index 8d3a218..efaf9a7 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_debugfs.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_debugfs.c
@@ -37,20 +37,6 @@ static struct dentry *ixgbe_dbg_root;
static char ixgbe_dbg_reg_ops_buf[256] = "";
/**
- * ixgbe_dbg_reg_ops_open - prep the debugfs pokee data item when opened
- * @inode: inode that was opened
- * @filp: file info
- *
- * Stash the adapter pointer hiding in the inode into the file pointer where
- * we can find it later in the read and write calls
- **/
-static int ixgbe_dbg_reg_ops_open(struct inode *inode, struct file *filp)
-{
- filp->private_data = inode->i_private;
- return 0;
-}
-
-/**
* ixgbe_dbg_reg_ops_read - read for reg_ops datum
* @filp: the opened file
* @buffer: where to write the data for the user to read
@@ -142,7 +128,7 @@ static ssize_t ixgbe_dbg_reg_ops_write(struct file *filp,
static const struct file_operations ixgbe_dbg_reg_ops_fops = {
.owner = THIS_MODULE,
- .open = ixgbe_dbg_reg_ops_open,
+ .open = simple_open,
.read = ixgbe_dbg_reg_ops_read,
.write = ixgbe_dbg_reg_ops_write,
};
@@ -150,20 +136,6 @@ static const struct file_operations ixgbe_dbg_reg_ops_fops = {
static char ixgbe_dbg_netdev_ops_buf[256] = "";
/**
- * ixgbe_dbg_netdev_ops_open - prep the debugfs netdev_ops data item
- * @inode: inode that was opened
- * @filp: file info
- *
- * Stash the adapter pointer hiding in the inode into the file pointer
- * where we can find it later in the read and write calls
- **/
-static int ixgbe_dbg_netdev_ops_open(struct inode *inode, struct file *filp)
-{
- filp->private_data = inode->i_private;
- return 0;
-}
-
-/**
* ixgbe_dbg_netdev_ops_read - read for netdev_ops datum
* @filp: the opened file
* @buffer: where to write the data for the user to read
@@ -238,7 +210,7 @@ static ssize_t ixgbe_dbg_netdev_ops_write(struct file *filp,
static const struct file_operations ixgbe_dbg_netdev_ops_fops = {
.owner = THIS_MODULE,
- .open = ixgbe_dbg_netdev_ops_open,
+ .open = simple_open,
.read = ixgbe_dbg_netdev_ops_read,
.write = ixgbe_dbg_netdev_ops_write,
};
^ permalink raw reply related
* Re: [PATCH] xfrm: use ISO C standard for array in linux/xfrm.h
From: Steffen Klassert @ 2012-10-17 8:27 UTC (permalink / raw)
To: Nicolas Dichtel; +Cc: herbert, netdev, davem
In-Reply-To: <507E6362.1080009@6wind.com>
On Wed, Oct 17, 2012 at 09:50:58AM +0200, Nicolas Dichtel wrote:
> We got a problem with "ip xfrm state add" when compiled with gcc 4.4.6.
> Error was "*** buffer overflow detected ***: ip terminated", because
> when we try to copy the key in struct xfrm_algo, the function
> strncpy() calls some builtin checks about the size of the
> destination buffer, which is 0. With the standard notation, there is
> no problem.
This is likely to be a gcc FORITFY bug. Try to build iproute2 with
the -D_FORTIFY_SOURCE=0 flag.
^ permalink raw reply
* [PATCH] netfilter: xt_TEE: dont use destination address found in header
From: Eric Dumazet @ 2012-10-17 8:33 UTC (permalink / raw)
To: Pablo Neira Ayuso
Cc: Torsten Luettgert, netfilter-devel, netdev, Julian Anastasov
In-Reply-To: <20121017081551.GA12998@1984>
From-off-by: Eric Dumazet <edumazet@google.com>
Torsten Luettgert bisected TEE regression starting with commit
f8126f1d5136be1 (ipv4: Adjust semantics of rt->rt_gateway.)
The problem is that it tries to ARP-lookup the original destination
address of the forwarded packet, not the address of the gateway.
Fix this using FLOWI_FLAG_KNOWN_NH Julian added in commit
c92b96553a80c1 (ipv4: Add FLOWI_FLAG_KNOWN_NH), so that known
nexthop (info->gw.ip) has preference on resolving.
Reported-by: Torsten Luettgert <ml-netfilter@enda.eu>
Bisected-by: Torsten Luettgert <ml-netfilter@enda.eu>
Tested-by: Torsten Luettgert <ml-netfilter@enda.eu>
Cc: Julian Anastasov <ja@ssi.bg>
Signed-off-by: Eric Dumazet <edumazet@google.com>
---
net/netfilter/xt_TEE.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/net/netfilter/xt_TEE.c b/net/netfilter/xt_TEE.c
index ee2e5bc..bd93e51 100644
--- a/net/netfilter/xt_TEE.c
+++ b/net/netfilter/xt_TEE.c
@@ -70,6 +70,7 @@ tee_tg_route4(struct sk_buff *skb, const struct xt_tee_tginfo *info)
fl4.daddr = info->gw.ip;
fl4.flowi4_tos = RT_TOS(iph->tos);
fl4.flowi4_scope = RT_SCOPE_UNIVERSE;
+ fl4.flowi4_flags = FLOWI_FLAG_KNOWN_NH;
rt = ip_route_output_key(net, &fl4);
if (IS_ERR(rt))
return false;
^ permalink raw reply related
* Re: [PATCH net] netfilter: nf_conntrack: fix rt_gateway checks for h323
From: Julian Anastasov @ 2012-10-17 9:03 UTC (permalink / raw)
To: Pablo Neira Ayuso; +Cc: netfilter-devel, netdev, zhaojingmin
In-Reply-To: <20121016235534.GA2805@1984>
Hello Pablo,
On Wed, 17 Oct 2012, Pablo Neira Ayuso wrote:
> Hi Julian,
>
> On Wed, Oct 10, 2012 at 02:00:47AM +0300, Julian Anastasov wrote:
> > After the change "Adjust semantics of rt->rt_gateway"
> > (commit f8126f1d51) we should properly match the nexthop when
> > destinations are directly connected because rt_gateway can be 0.
> >
> > Signed-off-by: Julian Anastasov <ja@ssi.bg>
> > ---
> >
> > This patch needs a closer look from the Netfilter team.
> >
> > It restores the check as it was committed originally,
> > i.e. to compare nexthops. I'm not sure what is the desired logic,
> > it can depend on the following:
> >
> > - two directly connected hosts (rt_gateway=0) can be from different
> > subnets or not
> >
> > - one party A is the gateway (rt_gateway=0), another party uses
> > this gateway (rt_gateway=A)
> >
> > May be someone that knows this code better can comment
> > if the check should be different.
>
> Your patch gets it working like before David's change in the
> rt_gateway semantics.
>
> I think the H.323 helper is doing "its best" to handle the following
> call-forwarding scenario:
>
> 1) A calls B.
> 2) B replies to A that the alternate address is C.
> 3) A calls C.
>
> Now assume that:
>
> 1) all traffic between A and B goes through the firewall.
>
> and
>
> 2) all traffic between A and C don't go through the firewall.
>
> If you want a picture, see section 5.2 of this site:
>
> http://people.netfilter.org/zhaojingmin/h323_conntrack_nat_helper/#_Toc133598073
>
> That code below is trying to detect if A and C don't go through the
> firewall, just to skip the creation of one useless expectation (since
> they can communicate without going through the firewall).
>
> With the code below (function callforward_do_filter):
>
> a) if A and C are on-link, then:
> rt_nexthop(rt1, fl1.daddr) != rt_nexthop(rt2, fl2.daddr)
>
> Bad luck, we create the expectation even if we don't need it.
>
> b) if A and C are behind the same next hop:
> rt_nexthop(rt1, fl1.daddr) == rt_nexthop(rt2, fl2.daddr)
> We don't create the expectation.
>
> c) if A is on-link and C is behind next hop:
> rt_nexthop(rt1, fl1.daddr) != rt_nexthop(rt2, fl2.daddr)
>
> Bad luck again, we create the expectation again.
>
> This seems documented. We could make it better if we would have a way
> to guess that A and C do not need to communicate through the firewall.
>
> I'll take your patch, it leaves things just like it was before (which
> was not really good).
>
> Please, let me know if I'm missing anything. Thanks.
When created the patch I forgot that this file has
history also in net/ipv4/netfilter/ but anyways. It is
possible to add more checks, for example, checking for
same subnet with inet_addr_onlink when rt_gateway=0:
if (rt1->dst.dev == rt2->dst.dev &&
((rt1->rt_uses_gateway &&
rt1->rt_gateway == rt2->rt_gateway) ||
inet_addr_onlink(__in_dev_get_rcu(rt1->dst.dev),
src->ip, dst->ip)))
Note that rt_gateway can be non-0 even for
directly connected hosts when rt is not cached. That
is why the rt_uses_gateway check is used instead of
rt_gateway!=0.
But if creating expectation is considered harmless
then better to use just the rt_nexthop check because
checking for subnets is too risky, hosts can use different
subnet masks. By this way we reduce the risk to connect
internal hosts without expectation.
> > net/netfilter/nf_conntrack_h323_main.c | 3 ++-
> > 1 files changed, 2 insertions(+), 1 deletions(-)
> >
> > diff --git a/net/netfilter/nf_conntrack_h323_main.c b/net/netfilter/nf_conntrack_h323_main.c
> > index 1b30b0d..962795e 100644
> > --- a/net/netfilter/nf_conntrack_h323_main.c
> > +++ b/net/netfilter/nf_conntrack_h323_main.c
> > @@ -753,7 +753,8 @@ static int callforward_do_filter(const union nf_inet_addr *src,
> > flowi4_to_flowi(&fl1), false)) {
> > if (!afinfo->route(&init_net, (struct dst_entry **)&rt2,
> > flowi4_to_flowi(&fl2), false)) {
> > - if (rt1->rt_gateway == rt2->rt_gateway &&
> > + if (rt_nexthop(rt1, fl1.daddr) ==
> > + rt_nexthop(rt2, fl2.daddr) &&
> > rt1->dst.dev == rt2->dst.dev)
> > ret = 1;
> > dst_release(&rt2->dst);
> > --
> > 1.7.3.4
> >
> > --
Regards
--
Julian Anastasov <ja@ssi.bg>
^ permalink raw reply
* Re: [PATCH] xfrm: use ISO C standard for array in linux/xfrm.h
From: Nicolas Dichtel @ 2012-10-17 9:03 UTC (permalink / raw)
To: Steffen Klassert; +Cc: herbert, netdev, davem
In-Reply-To: <20121017082731.GB12671@secunet.com>
Le 17/10/2012 10:27, Steffen Klassert a écrit :
> On Wed, Oct 17, 2012 at 09:50:58AM +0200, Nicolas Dichtel wrote:
>> We got a problem with "ip xfrm state add" when compiled with gcc 4.4.6.
>> Error was "*** buffer overflow detected ***: ip terminated", because
>> when we try to copy the key in struct xfrm_algo, the function
>> strncpy() calls some builtin checks about the size of the
>> destination buffer, which is 0. With the standard notation, there is
>> no problem.
>
> This is likely to be a gcc FORITFY bug. Try to build iproute2 with
> the -D_FORTIFY_SOURCE=0 flag.
>
Right, it works. Thank you.
^ permalink raw reply
* Re: IPSEC maintainership...
From: Steffen Klassert @ 2012-10-17 9:13 UTC (permalink / raw)
To: David Miller; +Cc: netdev, herbert
In-Reply-To: <20121016.021435.220828722592000976.davem@davemloft.net>
On Tue, Oct 16, 2012 at 02:14:35AM -0400, David Miller wrote:
>
> If you want you can maintainer an IPSEC GIT tree, where you apply any
> submitted IPSEC changes.
I've set up two git tries at kernel.org, an 'ipsec' tree that tracks the
net tree and an 'ipsec-next' tree that tracks the net-next tree.
They are located at
git://git.kernel.org/pub/scm/linux/kernel/git/klassert/ipsec.git
git://git.kernel.org/pub/scm/linux/kernel/git/klassert/ipsec-next.git
Once the workflow for IPsec patches is established, I'll ask for
linux-next inclusion of the ipsec-next tree to get early stage
testing of the IPsec changes.
Regarding my own IPsec patches, I'll continue to send them to netdev
for review with the remark that I'll apply them to the appropriate
tree if nobody has objections.
^ permalink raw reply
* Re: regression between v3.0 and v3.3 in bringing up IPoIB devices in a bond at boot
From: Jiri Pirko @ 2012-10-17 10:13 UTC (permalink / raw)
To: Jay Vosburgh; +Cc: Jon Stanley, netdev, Andy Gospodarek, davem, kaber
In-Reply-To: <1143.1350449897@death.nxdomain>
Wed, Oct 17, 2012 at 06:58:17AM CEST, fubar@us.ibm.com wrote:
>Jon Stanley <jstanley@rmrf.net> wrote:
>
>>Firstly, I apologize that this report isn't as well-formed as it
>>should be (i.e. I can't point to a specific commit that broke it), but
>>I'll do my best failing that. The reason I can't be as specific as I'd
>>like is that starting at 3.1-rc1, I'd run into the original VLAN
>>problem that I was having, and the patchset required to fix it (the
>>patch just submitted, efc73f4b, and 9b361c1) won't apply cleanly to
>>older kernels and I'm not sure it's worth the (seemingly massive)
>>effort required to backport them to something I don't plan to use :)
>>
>>First, let me explain the configuration a little:
>>
>>bond0 -> eth0, eth1.100
>>bond1 -> ib0, ib1
>>
>>The first problem, which is now resolved, was that ib0 and ib1
>>wouldn't enslave at all because of the presence of VLAN0. I can now
>>get them to enslave manually (echo +ib0 > /sys/.../slaves) *if* the
>>bond is down. If the bond is up, I still get the "refused to change
>>device type", which is fairly expected (I think, but that could be the
>>problem here).
>>
>>However, if I leave the distro (stock RHEL except the kernel) to it's
>>own devices to bring it up at boot, I get the same symptoms as though
>>the master (bond1 in this case) isn't down ("refused to change device
>>type"). If I go back to v3.0, everything works fine. I've also
>>verified that it doesn't work on current mainline, and that it works
>>fine without any VLAN devices configured on bond0.
>
> I haven't debugged on a live kernel yet to verify this, but I
>think I see what may be happening. Basically, when the device is up,
>VID 0 has been added, and the VLAN code refuses to change type on a
>device with a VLAN configured (even VID 0).
>
> If the bond is down (really, has never been up) when the
>NETDEV_PRE_TYPE_CHANGE event happens, the vlan_device_event callback
>will exit, because dev->vlan_info is NULL (dev here is the bond). It's
>NULL because vlan_device_event will respond to a NETDEV_UP event by
>adding VID 0 to the device, which is what allocates the dev->vlan_info,
>so when the bond has never been up, there is no dev->vlan_info.
>
> Once the bond is up, when the NETDEV_PRE_TYPE_CHANGE event
>reaches vlan_device_event, it will not exit as before, but proceed into
>the switch, and the NETDEV_PRE_TYPE_CHANGE case always returns
>NOTIFY_BAD, which results in the "refused to change type" message.
>
> It (in theory) works with no VLANs on bond0 because then I'll
>guess that you have no VLANs at all, and therefore the 8021q module
>isn't loaded, and so the whole "VID 0" and vlan_device_event business
>doesn't take place.
>
> I think that if vlan_device_event instead uses the new &
>improved vlan_uses_dev() test instead of its current test, that may
>resolve the problem; here's a patch against current net that I have not
>tested at all, but may fix things. I'm not 100% sure this is the right
>thing to do, as it may result in IPoIB interfaces with VID 0 configured
>on them.
I strongly believe you are right Jay. I will look at this and fix it.
Thanks
Jiri
>
>diff --git a/net/8021q/vlan.c b/net/8021q/vlan.c
>index 9096bcb..57170fa 100644
>--- a/net/8021q/vlan.c
>+++ b/net/8021q/vlan.c
>@@ -342,7 +342,7 @@ static int vlan_device_event(struct notifier_block *unused, unsigned long event,
> }
>
> vlan_info = rtnl_dereference(dev->vlan_info);
>- if (!vlan_info)
>+ if (!vlan_info || !vlan_info->grp.nr_vlan_devs)
> goto out;
> grp = &vlan_info->grp;
>
>
> Comments?
>
> -J
>
>---
> -Jay Vosburgh, IBM Linux Technology Center, fubar@us.ibm.com
>
^ permalink raw reply
* Re: regression between v3.0 and v3.3 in bringing up IPoIB devices in a bond at boot
From: Jiri Pirko @ 2012-10-17 10:29 UTC (permalink / raw)
To: Jay Vosburgh; +Cc: Jon Stanley, netdev, Andy Gospodarek, davem, kaber
In-Reply-To: <1143.1350449897@death.nxdomain>
Wed, Oct 17, 2012 at 06:58:17AM CEST, fubar@us.ibm.com wrote:
>Jon Stanley <jstanley@rmrf.net> wrote:
>
>>Firstly, I apologize that this report isn't as well-formed as it
>>should be (i.e. I can't point to a specific commit that broke it), but
>>I'll do my best failing that. The reason I can't be as specific as I'd
>>like is that starting at 3.1-rc1, I'd run into the original VLAN
>>problem that I was having, and the patchset required to fix it (the
>>patch just submitted, efc73f4b, and 9b361c1) won't apply cleanly to
>>older kernels and I'm not sure it's worth the (seemingly massive)
>>effort required to backport them to something I don't plan to use :)
>>
>>First, let me explain the configuration a little:
>>
>>bond0 -> eth0, eth1.100
>>bond1 -> ib0, ib1
>>
>>The first problem, which is now resolved, was that ib0 and ib1
>>wouldn't enslave at all because of the presence of VLAN0. I can now
>>get them to enslave manually (echo +ib0 > /sys/.../slaves) *if* the
>>bond is down. If the bond is up, I still get the "refused to change
>>device type", which is fairly expected (I think, but that could be the
>>problem here).
>>
>>However, if I leave the distro (stock RHEL except the kernel) to it's
>>own devices to bring it up at boot, I get the same symptoms as though
>>the master (bond1 in this case) isn't down ("refused to change device
>>type"). If I go back to v3.0, everything works fine. I've also
>>verified that it doesn't work on current mainline, and that it works
>>fine without any VLAN devices configured on bond0.
>
> I haven't debugged on a live kernel yet to verify this, but I
>think I see what may be happening. Basically, when the device is up,
>VID 0 has been added, and the VLAN code refuses to change type on a
>device with a VLAN configured (even VID 0).
>
> If the bond is down (really, has never been up) when the
>NETDEV_PRE_TYPE_CHANGE event happens, the vlan_device_event callback
>will exit, because dev->vlan_info is NULL (dev here is the bond). It's
>NULL because vlan_device_event will respond to a NETDEV_UP event by
>adding VID 0 to the device, which is what allocates the dev->vlan_info,
>so when the bond has never been up, there is no dev->vlan_info.
>
> Once the bond is up, when the NETDEV_PRE_TYPE_CHANGE event
>reaches vlan_device_event, it will not exit as before, but proceed into
>the switch, and the NETDEV_PRE_TYPE_CHANGE case always returns
>NOTIFY_BAD, which results in the "refused to change type" message.
>
> It (in theory) works with no VLANs on bond0 because then I'll
>guess that you have no VLANs at all, and therefore the 8021q module
>isn't loaded, and so the whole "VID 0" and vlan_device_event business
>doesn't take place.
>
> I think that if vlan_device_event instead uses the new &
>improved vlan_uses_dev() test instead of its current test, that may
>resolve the problem; here's a patch against current net that I have not
>tested at all, but may fix things. I'm not 100% sure this is the right
>thing to do, as it may result in IPoIB interfaces with VID 0 configured
>on them.
>
>diff --git a/net/8021q/vlan.c b/net/8021q/vlan.c
>index 9096bcb..57170fa 100644
>--- a/net/8021q/vlan.c
>+++ b/net/8021q/vlan.c
>@@ -342,7 +342,7 @@ static int vlan_device_event(struct notifier_block *unused, unsigned long event,
> }
>
> vlan_info = rtnl_dereference(dev->vlan_info);
>- if (!vlan_info)
>+ if (!vlan_info || !vlan_info->grp.nr_vlan_devs)
This patch would prevent implicit vlan0 from removal. I'll rather put
vlan_uses_dev() check into NETDEV_PRE_TYPE_CHANGE case.
> goto out;
> grp = &vlan_info->grp;
>
>
> Comments?
>
> -J
>
>---
> -Jay Vosburgh, IBM Linux Technology Center, fubar@us.ibm.com
>
^ permalink raw reply
* [PATCH] Netfilter/IPVS updates for your net tree (3.7)
From: pablo @ 2012-10-17 10:38 UTC (permalink / raw)
To: netfilter-devel; +Cc: davem, netdev
From: Pablo Neira Ayuso <pablo@netfilter.org>
Hi David,
The following patchset contains Netfilter/IPVS updates for your
net tree, they are:
* Fix incorrect hooks for SNAT and DNAT (bug introduced in recent IPv6
NAT changes), from Elison Niven.
* Fix xt_TEE (got broken with recent rt_gateway semantic change),
from Eric Dumazet.
* Fix custom conntrack timeout policy attachment for IPv6, from myself.
* Always initialize ip_vs_timeout_user in case that TCP or UDP protocols
is disabled, from Arnd Bergmann.
Note that I had to pull from your tree to obtain:
(c92b96553a80c1 ipv4: Add FLOWI_FLAG_KNOWN_NH)
which was required for the xt_TEE fix.
You can pull these changes from:
git://1984.lsi.us.es/nf master
Thanks!
Arnd Bergmann (1):
ipvs: initialize returned data in do_ip_vs_get_ctl
Elison Niven (1):
netfilter: xt_nat: fix incorrect hooks for SNAT and DNAT
Eric Dumazet (1):
netfilter: xt_TEE: don't use destination address found in header
Pablo Neira Ayuso (1):
netfilter: xt_CT: fix timeout setting with IPv6
--
1.7.10.4
^ permalink raw reply
* [PATCH 2/4] netfilter: xt_CT: fix timeout setting with IPv6
From: pablo @ 2012-10-17 10:40 UTC (permalink / raw)
To: netfilter-devel; +Cc: davem, netdev
In-Reply-To: <1350470411-5793-1-git-send-email-pablo@netfilter.org>
From: Pablo Neira Ayuso <pablo@netfilter.org>
This patch fixes ip6tables and the CT target if it is used to set
some custom conntrack timeout policy for IPv6.
Use xt_ct_find_proto which already handles the ip6tables case for us.
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
net/netfilter/xt_CT.c | 10 ++++++----
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/net/netfilter/xt_CT.c b/net/netfilter/xt_CT.c
index 16c7125..ae7f5da 100644
--- a/net/netfilter/xt_CT.c
+++ b/net/netfilter/xt_CT.c
@@ -180,9 +180,9 @@ xt_ct_set_timeout(struct nf_conn *ct, const struct xt_tgchk_param *par,
typeof(nf_ct_timeout_find_get_hook) timeout_find_get;
struct ctnl_timeout *timeout;
struct nf_conn_timeout *timeout_ext;
- const struct ipt_entry *e = par->entryinfo;
struct nf_conntrack_l4proto *l4proto;
int ret = 0;
+ u8 proto;
rcu_read_lock();
timeout_find_get = rcu_dereference(nf_ct_timeout_find_get_hook);
@@ -192,9 +192,11 @@ xt_ct_set_timeout(struct nf_conn *ct, const struct xt_tgchk_param *par,
goto out;
}
- if (e->ip.invflags & IPT_INV_PROTO) {
+ proto = xt_ct_find_proto(par);
+ if (!proto) {
ret = -EINVAL;
- pr_info("You cannot use inversion on L4 protocol\n");
+ pr_info("You must specify a L4 protocol, and not use "
+ "inversions on it.\n");
goto out;
}
@@ -214,7 +216,7 @@ xt_ct_set_timeout(struct nf_conn *ct, const struct xt_tgchk_param *par,
/* Make sure the timeout policy matches any existing protocol tracker,
* otherwise default to generic.
*/
- l4proto = __nf_ct_l4proto_find(par->family, e->ip.proto);
+ l4proto = __nf_ct_l4proto_find(par->family, proto);
if (timeout->l4proto->l4proto != l4proto->l4proto) {
ret = -EINVAL;
pr_info("Timeout policy `%s' can only be used by L4 protocol "
--
1.7.10.4
^ permalink raw reply related
* [PATCH 4/4] netfilter: xt_TEE: don't use destination address found in header
From: pablo @ 2012-10-17 10:40 UTC (permalink / raw)
To: netfilter-devel; +Cc: davem, netdev
In-Reply-To: <1350470411-5793-1-git-send-email-pablo@netfilter.org>
From: Eric Dumazet <eric.dumazet@gmail.com>
Torsten Luettgert bisected TEE regression starting with commit
f8126f1d5136be1 (ipv4: Adjust semantics of rt->rt_gateway.)
The problem is that it tries to ARP-lookup the original destination
address of the forwarded packet, not the address of the gateway.
Fix this using FLOWI_FLAG_KNOWN_NH Julian added in commit
c92b96553a80c1 (ipv4: Add FLOWI_FLAG_KNOWN_NH), so that known
nexthop (info->gw.ip) has preference on resolving.
Reported-by: Torsten Luettgert <ml-netfilter@enda.eu>
Bisected-by: Torsten Luettgert <ml-netfilter@enda.eu>
Tested-by: Torsten Luettgert <ml-netfilter@enda.eu>
Cc: Julian Anastasov <ja@ssi.bg>
Signed-off-by: Eric Dumazet <edumazet@google.com>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
net/netfilter/xt_TEE.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/net/netfilter/xt_TEE.c b/net/netfilter/xt_TEE.c
index ee2e5bc..bd93e51 100644
--- a/net/netfilter/xt_TEE.c
+++ b/net/netfilter/xt_TEE.c
@@ -70,6 +70,7 @@ tee_tg_route4(struct sk_buff *skb, const struct xt_tee_tginfo *info)
fl4.daddr = info->gw.ip;
fl4.flowi4_tos = RT_TOS(iph->tos);
fl4.flowi4_scope = RT_SCOPE_UNIVERSE;
+ fl4.flowi4_flags = FLOWI_FLAG_KNOWN_NH;
rt = ip_route_output_key(net, &fl4);
if (IS_ERR(rt))
return false;
--
1.7.10.4
^ permalink raw reply related
* [PATCH 1/4] ipvs: initialize returned data in do_ip_vs_get_ctl
From: pablo @ 2012-10-17 10:40 UTC (permalink / raw)
To: netfilter-devel; +Cc: davem, netdev
From: Arnd Bergmann <arnd@arndb.de>
As reported by a gcc warning, the do_ip_vs_get_ctl does not initalize
all the members of the ip_vs_timeout_user structure it returns if
at least one of the TCP or UDP protocols is disabled for ipvs.
This makes sure that the data is always initialized, before it is
returned as a response to IPVS_CMD_GET_CONFIG or printed as a
debug message in IPVS_CMD_SET_CONFIG.
Without this patch, building ARM ixp4xx_defconfig results in:
net/netfilter/ipvs/ip_vs_ctl.c: In function 'ip_vs_genl_set_cmd':
net/netfilter/ipvs/ip_vs_ctl.c:2238:47: warning: 't.udp_timeout' may be used uninitialized in this function [-Wuninitialized]
net/netfilter/ipvs/ip_vs_ctl.c:3322:28: note: 't.udp_timeout' was declared here
net/netfilter/ipvs/ip_vs_ctl.c:2238:47: warning: 't.tcp_fin_timeout' may be used uninitialized in this function [-Wuninitialized]
net/netfilter/ipvs/ip_vs_ctl.c:3322:28: note: 't.tcp_fin_timeout' was declared here
net/netfilter/ipvs/ip_vs_ctl.c:2238:47: warning: 't.tcp_timeout' may be used uninitialized in this function [-Wuninitialized]
net/netfilter/ipvs/ip_vs_ctl.c:3322:28: note: 't.tcp_timeout' was declared here
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Acked-by: Julian Anastasov <ja@ssi.bg>
Signed-off-by: Simon Horman <horms@verge.net.au>
---
net/netfilter/ipvs/ip_vs_ctl.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/net/netfilter/ipvs/ip_vs_ctl.c b/net/netfilter/ipvs/ip_vs_ctl.c
index 7e7198b..c4ee437 100644
--- a/net/netfilter/ipvs/ip_vs_ctl.c
+++ b/net/netfilter/ipvs/ip_vs_ctl.c
@@ -2589,6 +2589,8 @@ __ip_vs_get_timeouts(struct net *net, struct ip_vs_timeout_user *u)
struct ip_vs_proto_data *pd;
#endif
+ memset(u, 0, sizeof (*u));
+
#ifdef CONFIG_IP_VS_PROTO_TCP
pd = ip_vs_proto_data_get(net, IPPROTO_TCP);
u->tcp_timeout = pd->timeout_table[IP_VS_TCP_S_ESTABLISHED] / HZ;
@@ -2766,7 +2768,6 @@ do_ip_vs_get_ctl(struct sock *sk, int cmd, void __user *user, int *len)
{
struct ip_vs_timeout_user t;
- memset(&t, 0, sizeof(t));
__ip_vs_get_timeouts(net, &t);
if (copy_to_user(user, &t, sizeof(t)) != 0)
ret = -EFAULT;
--
1.7.10.4
^ permalink raw reply related
* [PATCH 3/4] netfilter: xt_nat: fix incorrect hooks for SNAT and DNAT targets
From: pablo @ 2012-10-17 10:40 UTC (permalink / raw)
To: netfilter-devel; +Cc: davem, netdev
In-Reply-To: <1350470411-5793-1-git-send-email-pablo@netfilter.org>
From: Elison Niven <elison.niven@cyberoam.com>
In (c7232c9 netfilter: add protocol independent NAT core), the
hooks were accidentally modified:
SNAT hooks are POST_ROUTING and LOCAL_IN (before it was LOCAL_OUT).
DNAT hooks are PRE_ROUTING and LOCAL_OUT (before it was LOCAL_IN).
Signed-off-by: Elison Niven <elison.niven@cyberoam.com>
Signed-off-by: Sanket Shah <sanket.shah@cyberoam.com>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
net/netfilter/xt_nat.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/net/netfilter/xt_nat.c b/net/netfilter/xt_nat.c
index 81aafa8..bea7464 100644
--- a/net/netfilter/xt_nat.c
+++ b/net/netfilter/xt_nat.c
@@ -111,7 +111,7 @@ static struct xt_target xt_nat_target_reg[] __read_mostly = {
.family = NFPROTO_IPV4,
.table = "nat",
.hooks = (1 << NF_INET_POST_ROUTING) |
- (1 << NF_INET_LOCAL_OUT),
+ (1 << NF_INET_LOCAL_IN),
.me = THIS_MODULE,
},
{
@@ -123,7 +123,7 @@ static struct xt_target xt_nat_target_reg[] __read_mostly = {
.family = NFPROTO_IPV4,
.table = "nat",
.hooks = (1 << NF_INET_PRE_ROUTING) |
- (1 << NF_INET_LOCAL_IN),
+ (1 << NF_INET_LOCAL_OUT),
.me = THIS_MODULE,
},
{
@@ -133,7 +133,7 @@ static struct xt_target xt_nat_target_reg[] __read_mostly = {
.targetsize = sizeof(struct nf_nat_range),
.table = "nat",
.hooks = (1 << NF_INET_POST_ROUTING) |
- (1 << NF_INET_LOCAL_OUT),
+ (1 << NF_INET_LOCAL_IN),
.me = THIS_MODULE,
},
{
@@ -143,7 +143,7 @@ static struct xt_target xt_nat_target_reg[] __read_mostly = {
.targetsize = sizeof(struct nf_nat_range),
.table = "nat",
.hooks = (1 << NF_INET_PRE_ROUTING) |
- (1 << NF_INET_LOCAL_IN),
+ (1 << NF_INET_LOCAL_OUT),
.me = THIS_MODULE,
},
};
--
1.7.10.4
^ permalink raw reply related
* Re: unresponsive vlan on top of bond with fail_over_mac=active
From: Michal Kubecek @ 2012-10-17 11:08 UTC (permalink / raw)
To: Jay Vosburgh; +Cc: netdev, Andy Gospodarek
In-Reply-To: <29107.1350002028@death.nxdomain>
On Thu, Oct 11, 2012 at 05:33:48PM -0700, Jay Vosburgh wrote:
>
> Have you tried the "follow" setting to fail_over_mac?
Yes, it works both for me and for the customer. They actually tried it
themselves before reporting the bug. I explained that "follow" is more
suitable also for other reasons and I hope they will accept it.
> What kernel are you looking at? In current mainline, bonding
> does have bond_set_multicast_list as ndo_set_rx_mode, although it
> doesn't propagate unicast address information, only multicast. I
> believe this has been the case for a long time.
I'm sorry, you are right. What I was looking at was actually 3.0 where
.ndo_set_multicast_list is used rather than .ndo_set_rx_mode (changed by
commit afc4b13d in v3.2-rc1).
Thank you for your help,
Michal Kubeček
^ permalink raw reply
* [patch net] vlan: allow to change type when no vlan device is hooked on netdev
From: Jiri Pirko @ 2012-10-17 11:37 UTC (permalink / raw)
To: netdev; +Cc: davem, fubar, andy, kaber, jstanley
vlan_info might be present but still no vlan devices might be there.
That is in case of vlan0 automatically added.
So in that case, allow to change netdev type.
Reported-by: Jon Stanley <jstanley@rmrf.net>
Signed-off-by: Jiri Pirko <jiri@resnulli.us>
---
net/8021q/vlan.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/net/8021q/vlan.c b/net/8021q/vlan.c
index 9096bcb..ee07072 100644
--- a/net/8021q/vlan.c
+++ b/net/8021q/vlan.c
@@ -463,7 +463,9 @@ static int vlan_device_event(struct notifier_block *unused, unsigned long event,
case NETDEV_PRE_TYPE_CHANGE:
/* Forbid underlaying device to change its type. */
- return NOTIFY_BAD;
+ if (vlan_uses_dev(dev))
+ return NOTIFY_BAD;
+ break;
case NETDEV_NOTIFY_PEERS:
case NETDEV_BONDING_FAILOVER:
--
1.7.11.4
^ permalink raw reply related
* Resending patch: [PATCH] net/smsc911x: check chip ready bit with swapped data
From: Kamlakant Patel @ 2012-10-17 11:40 UTC (permalink / raw)
To: netdev; +Cc: Kamlakant Patel
The chip ready check added by the commit 3ac3546e [Always wait for
the chip to be ready] does not work when the registers are swapped.
Since we have not programmed WORD_SWAP register before chip ready
bit check, we need to check for swapped register value as well.
Signed-off-by: Kamlakant Patel <kamlakant.patel@broadcom.com>
---
drivers/net/ethernet/smsc/smsc911x.c | 10 ++++++++--
1 files changed, 8 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/smsc/smsc911x.c b/drivers/net/ethernet/smsc/smsc911x.c
index 62d1baf..419bfb9 100644
--- a/drivers/net/ethernet/smsc/smsc911x.c
+++ b/drivers/net/ethernet/smsc/smsc911x.c
@@ -2110,7 +2110,7 @@ static void __devinit smsc911x_read_mac_address(struct net_device *dev)
static int __devinit smsc911x_init(struct net_device *dev)
{
struct smsc911x_data *pdata = netdev_priv(dev);
- unsigned int byte_test;
+ unsigned int byte_test, mask;
unsigned int to = 100;
SMSC_TRACE(pdata, probe, "Driver Parameters:");
@@ -2130,9 +2130,15 @@ static int __devinit smsc911x_init(struct net_device *dev)
/*
* poll the READY bit in PMT_CTRL. Any other access to the device is
* forbidden while this bit isn't set. Try for 100ms
+ *
+ * The register read may be swapped, since we have not programmed
+ * WORD_SWAP register, check for swapped register value as well
*/
- while (!(smsc911x_reg_read(pdata, PMT_CTRL) & PMT_CTRL_READY_) && --to)
+
+ mask = PMT_CTRL_READY_ | swahw32(PMT_CTRL_READY_);
+ while (!(smsc911x_reg_read(pdata, PMT_CTRL) & mask) && --to)
udelay(1000);
+
if (to == 0) {
pr_err("Device not READY in 100ms aborting\n");
return -ENODEV;
--
1.7.6
^ permalink raw reply related
* Business-Darlehen
From: supreme finances. @ 2012-10-17 11:38 UTC (permalink / raw)
Wir bieten legitimen Darlehen wie persönliche Kredite & Business Darlehen ohne Sicherheiten mit maximaler Garantie,
€ 5.000 bis € 90.000.000, Zinssätze niedrig wie 3% in 1 Jahr bis 40 Jahre Rückzahlungsfrist überall in der Welt
^ permalink raw reply
* Re: [PATCH net] netfilter: nf_conntrack: fix rt_gateway checks for h323
From: Pablo Neira Ayuso @ 2012-10-17 12:39 UTC (permalink / raw)
To: Julian Anastasov; +Cc: netfilter-devel, netdev, zhaojingmin
In-Reply-To: <alpine.LFD.2.00.1210171111150.1679@ja.ssi.bg>
On Wed, Oct 17, 2012 at 12:03:05PM +0300, Julian Anastasov wrote:
[...]
> On Wed, 17 Oct 2012, Pablo Neira Ayuso wrote:
[...]
> > On Wed, Oct 10, 2012 at 02:00:47AM +0300, Julian Anastasov wrote:
> > > After the change "Adjust semantics of rt->rt_gateway"
> > > (commit f8126f1d51) we should properly match the nexthop when
> > > destinations are directly connected because rt_gateway can be 0.
> > >
> > > Signed-off-by: Julian Anastasov <ja@ssi.bg>
> > > ---
> > >
> > > This patch needs a closer look from the Netfilter team.
> > >
> > > It restores the check as it was committed originally,
> > > i.e. to compare nexthops. I'm not sure what is the desired logic,
> > > it can depend on the following:
> > >
> > > - two directly connected hosts (rt_gateway=0) can be from different
> > > subnets or not
> > >
> > > - one party A is the gateway (rt_gateway=0), another party uses
> > > this gateway (rt_gateway=A)
> > >
> > > May be someone that knows this code better can comment
> > > if the check should be different.
> >
> > Your patch gets it working like before David's change in the
> > rt_gateway semantics.
> >
> > I think the H.323 helper is doing "its best" to handle the following
> > call-forwarding scenario:
> >
> > 1) A calls B.
> > 2) B replies to A that the alternate address is C.
> > 3) A calls C.
> >
> > Now assume that:
> >
> > 1) all traffic between A and B goes through the firewall.
> >
> > and
> >
> > 2) all traffic between A and C don't go through the firewall.
> >
> > If you want a picture, see section 5.2 of this site:
> >
> > http://people.netfilter.org/zhaojingmin/h323_conntrack_nat_helper/#_Toc133598073
> >
> > That code below is trying to detect if A and C don't go through the
> > firewall, just to skip the creation of one useless expectation (since
> > they can communicate without going through the firewall).
> >
> > With the code below (function callforward_do_filter):
> >
> > a) if A and C are on-link, then:
> > rt_nexthop(rt1, fl1.daddr) != rt_nexthop(rt2, fl2.daddr)
> >
> > Bad luck, we create the expectation even if we don't need it.
> >
> > b) if A and C are behind the same next hop:
> > rt_nexthop(rt1, fl1.daddr) == rt_nexthop(rt2, fl2.daddr)
> > We don't create the expectation.
> >
> > c) if A is on-link and C is behind next hop:
> > rt_nexthop(rt1, fl1.daddr) != rt_nexthop(rt2, fl2.daddr)
> >
> > Bad luck again, we create the expectation again.
> >
> > This seems documented. We could make it better if we would have a way
> > to guess that A and C do not need to communicate through the firewall.
> >
> > I'll take your patch, it leaves things just like it was before (which
> > was not really good).
> >
> > Please, let me know if I'm missing anything. Thanks.
>
> When created the patch I forgot that this file has
> history also in net/ipv4/netfilter/ but anyways. It is
> possible to add more checks, for example, checking for
> same subnet with inet_addr_onlink when rt_gateway=0:
>
> if (rt1->dst.dev == rt2->dst.dev &&
> ((rt1->rt_uses_gateway &&
> rt1->rt_gateway == rt2->rt_gateway) ||
> inet_addr_onlink(__in_dev_get_rcu(rt1->dst.dev),
> src->ip, dst->ip)))
>
> Note that rt_gateway can be non-0 even for
> directly connected hosts when rt is not cached. That
> is why the rt_uses_gateway check is used instead of
> rt_gateway!=0.
>
> But if creating expectation is considered harmless
> then better to use just the rt_nexthop check because
> checking for subnets is too risky, hosts can use different
> subnet masks. By this way we reduce the risk to connect
> internal hosts without expectation.
It creates an expection for the IP parsed as result of the call
forwarding that will expire after 240 seconds. So it consumes memory
but it will expire after that time.
I prefer not to modify this code unless someone using the H.323 helper
reports a problem.
Thanks for your comments. I'll take your patch and extend the
description a bit to explain the current situation of that chunk of
code for the history.
> > > net/netfilter/nf_conntrack_h323_main.c | 3 ++-
> > > 1 files changed, 2 insertions(+), 1 deletions(-)
> > >
> > > diff --git a/net/netfilter/nf_conntrack_h323_main.c b/net/netfilter/nf_conntrack_h323_main.c
> > > index 1b30b0d..962795e 100644
> > > --- a/net/netfilter/nf_conntrack_h323_main.c
> > > +++ b/net/netfilter/nf_conntrack_h323_main.c
> > > @@ -753,7 +753,8 @@ static int callforward_do_filter(const union nf_inet_addr *src,
> > > flowi4_to_flowi(&fl1), false)) {
> > > if (!afinfo->route(&init_net, (struct dst_entry **)&rt2,
> > > flowi4_to_flowi(&fl2), false)) {
> > > - if (rt1->rt_gateway == rt2->rt_gateway &&
> > > + if (rt_nexthop(rt1, fl1.daddr) ==
> > > + rt_nexthop(rt2, fl2.daddr) &&
> > > rt1->dst.dev == rt2->dst.dev)
> > > ret = 1;
> > > dst_release(&rt2->dst);
> > > --
> > > 1.7.3.4
> > >
> > > --
>
> Regards
>
> --
> Julian Anastasov <ja@ssi.bg>
^ permalink raw reply
* [PATCH v0 1/4] cgroup: net_prio: Mark local used function static
From: Daniel Wagner @ 2012-10-17 13:04 UTC (permalink / raw)
To: netdev-u79uwXL29TY76Z2rM5mHXA, cgroups-u79uwXL29TY76Z2rM5mHXA
Cc: Daniel Wagner, David S. Miller, John Fastabend, Li Zefan,
Neil Horman, Tejun Heo
In-Reply-To: <1350479078-29361-1-git-send-email-wagi-kQCPcA+X3s7YtjvyW6yDsg@public.gmane.org>
From: Daniel Wagner <daniel.wagner-98C5kh4wR6ohFhg+JK9F0w@public.gmane.org>
net_prio_attach() is only access via cgroup_subsys callbacks,
therefore we can reduce the visibility of this function.
Signed-off-by: Daniel Wagner <daniel.wagner-98C5kh4wR6ohFhg+JK9F0w@public.gmane.org>
Cc: "David S. Miller" <davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org>
Cc: John Fastabend <john.r.fastabend-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
Cc: Li Zefan <lizefan-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
Cc: Neil Horman <nhorman-2XuSBdqkA4R54TAoqtyWWQ@public.gmane.org>
Cc: Tejun Heo <tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
Cc: <netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>
Cc: <cgroups-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>
---
net/core/netprio_cgroup.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/core/netprio_cgroup.c b/net/core/netprio_cgroup.c
index 4a83fb3..5ac384e 100644
--- a/net/core/netprio_cgroup.c
+++ b/net/core/netprio_cgroup.c
@@ -239,7 +239,7 @@ out_free_devname:
return ret;
}
-void net_prio_attach(struct cgroup *cgrp, struct cgroup_taskset *tset)
+static void net_prio_attach(struct cgroup *cgrp, struct cgroup_taskset *tset)
{
struct task_struct *p;
--
1.7.11.4
^ permalink raw reply related
* [PATCH v0 2/4] cgroup: net_cls: Fix local variable type decleration
From: Daniel Wagner @ 2012-10-17 13:04 UTC (permalink / raw)
To: netdev-u79uwXL29TY76Z2rM5mHXA, cgroups-u79uwXL29TY76Z2rM5mHXA
Cc: Daniel Wagner, David S. Miller, Li Zefan, Tejun Heo
In-Reply-To: <1350479078-29361-1-git-send-email-wagi-kQCPcA+X3s7YtjvyW6yDsg@public.gmane.org>
From: Daniel Wagner <daniel.wagner-98C5kh4wR6ohFhg+JK9F0w@public.gmane.org>
The classid type used throughout the kernel is u32.
Signed-off-by: Daniel Wagner <daniel.wagner-98C5kh4wR6ohFhg+JK9F0w@public.gmane.org>
Cc: "David S. Miller" <davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org>
Cc: Li Zefan <lizefan-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
Cc: Tejun Heo <tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
Cc: <netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>
Cc: <cgroups-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>
---
include/net/cls_cgroup.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/net/cls_cgroup.h b/include/net/cls_cgroup.h
index b6a6eeb..0fee061 100644
--- a/include/net/cls_cgroup.h
+++ b/include/net/cls_cgroup.h
@@ -29,7 +29,7 @@ extern void sock_update_classid(struct sock *sk);
#if IS_BUILTIN(CONFIG_NET_CLS_CGROUP)
static inline u32 task_cls_classid(struct task_struct *p)
{
- int classid;
+ u32 classid;
if (in_interrupt())
return 0;
--
1.7.11.4
^ permalink raw reply related
* [PATCH v0 4/4] cgroup: net_cls: Rework update socket logic
From: Daniel Wagner @ 2012-10-17 13:04 UTC (permalink / raw)
To: netdev-u79uwXL29TY76Z2rM5mHXA, cgroups-u79uwXL29TY76Z2rM5mHXA
Cc: Daniel Wagner, Li Zefan, David S. Miller, Michael S. Tsirkin,
Jamal Hadi Salim, Joe Perches, John Fastabend, Neil Horman,
Stanislav Kinsbursky, Tejun Heo
In-Reply-To: <1350479078-29361-1-git-send-email-wagi-kQCPcA+X3s7YtjvyW6yDsg@public.gmane.org>
From: Daniel Wagner <daniel.wagner-98C5kh4wR6ohFhg+JK9F0w@public.gmane.org>
The cgroup logic part of net_cls is very similar as the one in
net_prio. Let's stream line the net_cls logic with the net_prio one.
The net_prio update logic was changed by following commit (note there
were some changes necessary later on)
commit 406a3c638ce8b17d9704052c07955490f732c2b8
Author: John Fastabend <john.r.fastabend-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
Date: Fri Jul 20 10:39:25 2012 +0000
net: netprio_cgroup: rework update socket logic
Instead of updating the sk_cgrp_prioidx struct field on every send
this only updates the field when a task is moved via cgroup
infrastructure.
This allows sockets that may be used by a kernel worker thread
to be managed. For example in the iscsi case today a user can
put iscsid in a netprio cgroup and control traffic will be sent
with the correct sk_cgrp_prioidx value set but as soon as data
is sent the kernel worker thread isssues a send and sk_cgrp_prioidx
is updated with the kernel worker threads value which is the
default case.
It seems more correct to only update the field when the user
explicitly sets it via control group infrastructure. This allows
the users to manage sockets that may be used with other threads.
Since classid is now updated when the task is moved between the
cgroups, we don't have to call sock_update_classid() from various
places to ensure we always using the latest classid value.
Signed-off-by: Daniel Wagner <daniel.wagner-98C5kh4wR6ohFhg+JK9F0w@public.gmane.org>
Cc: Li Zefan <lizefan-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
Cc: "David S. Miller" <davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org>
Cc: "Michael S. Tsirkin" <mst-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
Cc: Jamal Hadi Salim <jhs-jkUAjuhPggJWk0Htik3J/w@public.gmane.org>
Cc: Joe Perches <joe-6d6DIl74uiNBDgjK7y7TUQ@public.gmane.org>
Cc: John Fastabend <john.r.fastabend-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
Cc: Neil Horman <nhorman-2XuSBdqkA4R54TAoqtyWWQ@public.gmane.org>
Cc: Stanislav Kinsbursky <skinsbursky-bzQdu9zFT3WakBO8gow8eQ@public.gmane.org>
Cc: Tejun Heo <tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
Cc: <netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>
Cc: <cgroups-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>
---
drivers/net/tun.c | 3 ---
net/sched/cls_cgroup.c | 38 ++++++++++++++++++++++++++++++++++++++
net/socket.c | 8 --------
3 files changed, 38 insertions(+), 11 deletions(-)
diff --git a/drivers/net/tun.c b/drivers/net/tun.c
index e4858b2..3157519 100644
--- a/drivers/net/tun.c
+++ b/drivers/net/tun.c
@@ -68,7 +68,6 @@
#include <net/netns/generic.h>
#include <net/rtnetlink.h>
#include <net/sock.h>
-#include <net/cls_cgroup.h>
#include <asm/uaccess.h>
@@ -587,8 +586,6 @@ static struct sk_buff *tun_alloc_skb(struct tun_struct *tun,
struct sk_buff *skb;
int err;
- sock_update_classid(sk, current);
-
/* Under a page? Don't bother with paged skb. */
if (prepad + len < PAGE_SIZE || !linear)
linear = len;
diff --git a/net/sched/cls_cgroup.c b/net/sched/cls_cgroup.c
index 2ecde22..789edb8 100644
--- a/net/sched/cls_cgroup.c
+++ b/net/sched/cls_cgroup.c
@@ -17,6 +17,7 @@
#include <linux/skbuff.h>
#include <linux/cgroup.h>
#include <linux/rcupdate.h>
+#include <linux/fdtable.h>
#include <net/rtnetlink.h>
#include <net/pkt_cls.h>
#include <net/sock.h>
@@ -53,6 +54,42 @@ static void cgrp_destroy(struct cgroup *cgrp)
kfree(cgrp_cls_state(cgrp));
}
+static void cgrp_attach(struct cgroup *cgrp, struct cgroup_taskset *tset)
+{
+ struct task_struct *p;
+
+ cgroup_taskset_for_each(p, cgrp, tset) {
+ unsigned int fd;
+ struct fdtable *fdt;
+ struct files_struct *files;
+
+ task_lock(p);
+ files = p->files;
+ if (!files) {
+ task_unlock(p);
+ continue;
+ }
+
+ spin_lock(&files->file_lock);
+ fdt = files_fdtable(files);
+ for (fd = 0; fd < fdt->max_fds; fd++) {
+ struct file *file;
+ struct socket *sock;
+ int err;
+
+ file = fcheck_files(files, fd);
+ if (!file)
+ continue;
+
+ sock = sock_from_file(file, &err);
+ if (sock)
+ sock_update_classid(sock->sk, p);
+ }
+ spin_unlock(&files->file_lock);
+ task_unlock(p);
+ }
+}
+
static u64 read_classid(struct cgroup *cgrp, struct cftype *cft)
{
return cgrp_cls_state(cgrp)->classid;
@@ -77,6 +114,7 @@ struct cgroup_subsys net_cls_subsys = {
.name = "net_cls",
.create = cgrp_create,
.destroy = cgrp_destroy,
+ .attach = cgrp_attach,
.subsys_id = net_cls_subsys_id,
.base_cftypes = ss_files,
.module = THIS_MODULE,
diff --git a/net/socket.c b/net/socket.c
index b38aff9..d296648 100644
--- a/net/socket.c
+++ b/net/socket.c
@@ -625,8 +625,6 @@ static inline int __sock_sendmsg_nosec(struct kiocb *iocb, struct socket *sock,
{
struct sock_iocb *si = kiocb_to_siocb(iocb);
- sock_update_classid(sock->sk, current);
-
si->sock = sock;
si->scm = NULL;
si->msg = msg;
@@ -789,8 +787,6 @@ static inline int __sock_recvmsg_nosec(struct kiocb *iocb, struct socket *sock,
{
struct sock_iocb *si = kiocb_to_siocb(iocb);
- sock_update_classid(sock->sk, current);
-
si->sock = sock;
si->scm = NULL;
si->msg = msg;
@@ -901,8 +897,6 @@ static ssize_t sock_splice_read(struct file *file, loff_t *ppos,
if (unlikely(!sock->ops->splice_read))
return -EINVAL;
- sock_update_classid(sock->sk, current);
-
return sock->ops->splice_read(sock, ppos, pipe, len, flags);
}
@@ -3421,8 +3415,6 @@ EXPORT_SYMBOL(kernel_setsockopt);
int kernel_sendpage(struct socket *sock, struct page *page, int offset,
size_t size, int flags)
{
- sock_update_classid(sock->sk, current);
-
if (sock->ops->sendpage)
return sock->ops->sendpage(sock, page, offset, size, flags);
--
1.7.11.4
^ permalink raw reply related
* [PATCH v0 0/4] Rework net_cls update socket logic
From: Daniel Wagner @ 2012-10-17 13:04 UTC (permalink / raw)
To: netdev, cgroups
Cc: Daniel Wagner, Li Zefan, David S. Miller, Michael S. Tsirkin,
Eric Dumazet, Glauber Costa, Jamal Hadi Salim, Joe Perches,
John Fastabend, Neil Horman, Stanislav Kinsbursky, Tejun Heo
From: Daniel Wagner <daniel.wagner@bmw-carit.de>
Hi,
This series updates net_cls to use the same socket update logic as
net_prio. With this change sock_update_classid() is not called from
recvmsg, sendmsg and friends anymore. This seems to be a good idea.
Credit goes to John Fastabend for this work.
The patches are against net-next.
cheers,
daniel
Cc: Li Zefan <lizefan@huawei.com>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: "Michael S. Tsirkin" <mst@redhat.com>
Cc: Eric Dumazet <edumazet@google.com>
Cc: Glauber Costa <glommer@parallels.com>
Cc: Jamal Hadi Salim <jhs@mojatatu.com>
Cc: Joe Perches <joe@perches.com>
Cc: John Fastabend <john.r.fastabend@intel.com>
Cc: Li Zefan <lizefan@huawei.com>
Cc: Neil Horman <nhorman@tuxdriver.com>
Cc: Stanislav Kinsbursky <skinsbursky@parallels.com>
Cc: Tejun Heo <tj@kernel.org>
Cc: <cgroups@vger.kernel.org>
Cc: <netdev@vger.kernel.org>
Daniel Wagner (4):
cgroup: net_prio: Mark local used function static
cgroup: net_cls: Fix local variable type decleration
cgroup: net_cls: Pass in task to sock_update_classid()
cgroup: net_cls: Rework update socket logic
drivers/net/tun.c | 3 ---
include/net/cls_cgroup.h | 4 ++--
net/core/netprio_cgroup.c | 2 +-
net/core/sock.c | 6 +++---
net/sched/cls_cgroup.c | 38 ++++++++++++++++++++++++++++++++++++++
net/socket.c | 8 --------
6 files changed, 44 insertions(+), 17 deletions(-)
--
1.7.11.4
^ permalink raw reply
* [PATCH v0 3/4] cgroup: net_cls: Pass in task to sock_update_classid()
From: Daniel Wagner @ 2012-10-17 13:04 UTC (permalink / raw)
To: netdev, cgroups
Cc: Daniel Wagner, David S. Miller, Michael S. Tsirkin, Eric Dumazet,
Glauber Costa, Joe Perches, Neil Horman, Stanislav Kinsbursky,
Tejun Heo
In-Reply-To: <1350479078-29361-1-git-send-email-wagi@monom.org>
From: Daniel Wagner <daniel.wagner@bmw-carit.de>
sock_update_classid() assumes that the update operation always are
applied on the current task. sock_update_classid() needs to know on
which tasks to work on in order to be able to migrate task between
cgroups using the struct cgroup_subsys attach() callback.
Signed-off-by: Daniel Wagner <daniel.wagner@bmw-carit.de>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: "Michael S. Tsirkin" <mst@redhat.com>
Cc: Eric Dumazet <edumazet@google.com>
Cc: Glauber Costa <glommer@parallels.com>
Cc: Joe Perches <joe@perches.com>
Cc: Neil Horman <nhorman@tuxdriver.com>
Cc: Stanislav Kinsbursky <skinsbursky@parallels.com>
Cc: Tejun Heo <tj@kernel.org>
Cc: <netdev@vger.kernel.org>
Cc: <cgroups@vger.kernel.org>
---
drivers/net/tun.c | 2 +-
include/net/cls_cgroup.h | 2 +-
net/core/sock.c | 6 +++---
net/socket.c | 8 ++++----
4 files changed, 9 insertions(+), 9 deletions(-)
diff --git a/drivers/net/tun.c b/drivers/net/tun.c
index 0873cdc..e4858b2 100644
--- a/drivers/net/tun.c
+++ b/drivers/net/tun.c
@@ -587,7 +587,7 @@ static struct sk_buff *tun_alloc_skb(struct tun_struct *tun,
struct sk_buff *skb;
int err;
- sock_update_classid(sk);
+ sock_update_classid(sk, current);
/* Under a page? Don't bother with paged skb. */
if (prepad + len < PAGE_SIZE || !linear)
diff --git a/include/net/cls_cgroup.h b/include/net/cls_cgroup.h
index 0fee061..571de6e 100644
--- a/include/net/cls_cgroup.h
+++ b/include/net/cls_cgroup.h
@@ -24,7 +24,7 @@ struct cgroup_cls_state
u32 classid;
};
-extern void sock_update_classid(struct sock *sk);
+extern void sock_update_classid(struct sock *sk, struct task_struct *task);
#if IS_BUILTIN(CONFIG_NET_CLS_CGROUP)
static inline u32 task_cls_classid(struct task_struct *p)
diff --git a/net/core/sock.c b/net/core/sock.c
index 8a146cf..68c2f3b 100644
--- a/net/core/sock.c
+++ b/net/core/sock.c
@@ -1214,12 +1214,12 @@ static void sk_prot_free(struct proto *prot, struct sock *sk)
#ifdef CONFIG_CGROUPS
#if IS_ENABLED(CONFIG_NET_CLS_CGROUP)
-void sock_update_classid(struct sock *sk)
+void sock_update_classid(struct sock *sk, struct task_struct *task)
{
u32 classid;
rcu_read_lock(); /* doing current task, which cannot vanish. */
- classid = task_cls_classid(current);
+ classid = task_cls_classid(task);
rcu_read_unlock();
if (classid != sk->sk_classid)
sk->sk_classid = classid;
@@ -1263,7 +1263,7 @@ struct sock *sk_alloc(struct net *net, int family, gfp_t priority,
sock_net_set(sk, get_net(net));
atomic_set(&sk->sk_wmem_alloc, 1);
- sock_update_classid(sk);
+ sock_update_classid(sk, current);
sock_update_netprioidx(sk, current);
}
diff --git a/net/socket.c b/net/socket.c
index 80dc7e8..b38aff9 100644
--- a/net/socket.c
+++ b/net/socket.c
@@ -625,7 +625,7 @@ static inline int __sock_sendmsg_nosec(struct kiocb *iocb, struct socket *sock,
{
struct sock_iocb *si = kiocb_to_siocb(iocb);
- sock_update_classid(sock->sk);
+ sock_update_classid(sock->sk, current);
si->sock = sock;
si->scm = NULL;
@@ -789,7 +789,7 @@ static inline int __sock_recvmsg_nosec(struct kiocb *iocb, struct socket *sock,
{
struct sock_iocb *si = kiocb_to_siocb(iocb);
- sock_update_classid(sock->sk);
+ sock_update_classid(sock->sk, current);
si->sock = sock;
si->scm = NULL;
@@ -901,7 +901,7 @@ static ssize_t sock_splice_read(struct file *file, loff_t *ppos,
if (unlikely(!sock->ops->splice_read))
return -EINVAL;
- sock_update_classid(sock->sk);
+ sock_update_classid(sock->sk, current);
return sock->ops->splice_read(sock, ppos, pipe, len, flags);
}
@@ -3421,7 +3421,7 @@ EXPORT_SYMBOL(kernel_setsockopt);
int kernel_sendpage(struct socket *sock, struct page *page, int offset,
size_t size, int flags)
{
- sock_update_classid(sock->sk);
+ sock_update_classid(sock->sk, current);
if (sock->ops->sendpage)
return sock->ops->sendpage(sock, page, offset, size, flags);
--
1.7.11.4
^ permalink raw reply related
* Re: [PATCH v0 3/4] cgroup: net_cls: Pass in task to sock_update_classid()
From: Eric Dumazet @ 2012-10-17 13:37 UTC (permalink / raw)
To: Daniel Wagner
Cc: netdev, cgroups, Daniel Wagner, David S. Miller,
Michael S. Tsirkin, Eric Dumazet, Glauber Costa, Joe Perches,
Neil Horman, Stanislav Kinsbursky, Tejun Heo
In-Reply-To: <1350479078-29361-4-git-send-email-wagi@monom.org>
On Wed, 2012-10-17 at 15:04 +0200, Daniel Wagner wrote:
> From: Daniel Wagner <daniel.wagner@bmw-carit.de>
>
> sock_update_classid() assumes that the update operation always are
> applied on the current task. sock_update_classid() needs to know on
> which tasks to work on in order to be able to migrate task between
> cgroups using the struct cgroup_subsys attach() callback.
>
...
>
> -extern void sock_update_classid(struct sock *sk);
> +extern void sock_update_classid(struct sock *sk, struct task_struct *task);
>
> #if IS_BUILTIN(CONFIG_NET_CLS_CGROUP)
> static inline u32 task_cls_classid(struct task_struct *p)
> diff --git a/net/core/sock.c b/net/core/sock.c
> index 8a146cf..68c2f3b 100644
> --- a/net/core/sock.c
> +++ b/net/core/sock.c
> @@ -1214,12 +1214,12 @@ static void sk_prot_free(struct proto *prot, struct sock *sk)
>
> #ifdef CONFIG_CGROUPS
> #if IS_ENABLED(CONFIG_NET_CLS_CGROUP)
> -void sock_update_classid(struct sock *sk)
> +void sock_update_classid(struct sock *sk, struct task_struct *task)
> {
> u32 classid;
>
> rcu_read_lock(); /* doing current task, which cannot vanish. */
> - classid = task_cls_classid(current);
> + classid = task_cls_classid(task);
> rcu_read_unlock();
Comment is now misleading (since task might not be current) and
rcu_read_lock()/rcu_read_unlock() protects nothing here, but avoid a
lockdep splat...
Hey task_cls_classid() has its own rcu protection since commit
3fb5a991916091a908d (cls_cgroup: Fix rcu lockdep warning)
So we can safely revert Paul commit (1144182a8757f2a1)
(We no longer need rcu_read_lock/unlock here)
(BTW net-next is not opened yet, its content outdated, although I like
your patch series !)
^ 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