* Re: [PATCH v2 net-next] tcp: avoid tx starvation by SYNACK packets
From: Eric Dumazet @ 2012-06-26 17:02 UTC (permalink / raw)
To: Hans Schillstrom
Cc: David Miller, subramanian.vijay@gmail.com, dave.taht@gmail.com,
netdev@vger.kernel.org, ncardwell@google.com, therbert@google.com,
brouer@redhat.com
In-Reply-To: <201206260734.33472.hans.schillstrom@ericsson.com>
On Tue, 2012-06-26 at 07:34 +0200, Hans Schillstrom wrote:
> This patch didn't give much in gain actually.
With a 100Mbps link it does.
With a 1Gbps link we are cpu bounded for sure.
> The big cycle consumer during a syn attack is SHA sum right now,
> so from that perspective it's better to add aes crypto (by using AES-NI)
> to the syn cookies instead of SHA sum. Even if only newer x86_64 can use it.
My dev machine is able to process ~280.000 SYN (and synack) per second
(tg3, mono queue), and sha_transform() takes ~10 % of the time according
to perf.
With David patch using jhash instead of SHA, I reach ~315.000 SYN per
second.
^ permalink raw reply
* Re: [PATCH 0/2] flexcan driver updates
From: Oliver Hartkopp @ 2012-06-26 16:55 UTC (permalink / raw)
To: Shawn Guo; +Cc: David S. Miller, Marc Kleine-Budde, netdev, linux-arm-kernel
In-Reply-To: <1340700563-8386-1-git-send-email-shawn.guo@linaro.org>
On 26.06.2012 10:49, Shawn Guo wrote:
> Here are a couple of flexcan driver/bindings updates, which are meant
> to get the driver more device tree friendly.
>
> Shawn Guo (2):
> net: flexcan: clock-frequency is optional for device tree probe
> net: flexcan: add transceiver switch gpio support
>
> .../devicetree/bindings/net/can/fsl-flexcan.txt | 6 ++++
> drivers/net/can/flexcan.c | 30 ++++++++++++++++++++
> 2 files changed, 36 insertions(+), 0 deletions(-)
>
Sorry Shawn, but your posting is pretty misplaced ...
Please check for the maintainers and mailing lists in the MAINTAINERS file
that match your suggested changes:
Documentation/devicetree/bindings/net/can/fsl-flexcan.txt =>
OPEN FIRMWARE AND FLATTENED DEVICE TREE
M: Grant Likely <grant.likely@secretlab.ca>
M: Rob Herring <rob.herring@calxeda.com>
L: devicetree-discuss@lists.ozlabs.org (moderated for non-subscribers)
W: http://fdt.secretlab.ca
T: git git://git.secretlab.ca/git/linux-2.6.git
S: Maintained
F: Documentation/devicetree <<<<<------------- !!!
F: drivers/of
F: include/linux/of*.h
K: of_get_property
K: of_match_table
drivers/net/can/flexcan.c =>
CAN NETWORK DRIVERS
M: Wolfgang Grandegger <wg@grandegger.com>
M: Marc Kleine-Budde <mkl@pengutronix.de>
L: linux-can@vger.kernel.org
W: http://gitorious.org/linux-can
T: git git://gitorious.org/linux-can/linux-can-next.git
S: Maintained
F: drivers/net/can/ <<<<<------------- !!!
F: include/linux/can/dev.h
F: include/linux/can/error.h
F: include/linux/can/netlink.h
F: include/linux/can/platform/
So please post your suggested changes for the flexcan driver on the mailing
lists linux-can@vger.kernel.org and devicetree-discuss@lists.ozlabs.org
Once the changes are discussed and accepted they can be pulled by Dave Miller
from the CAN development git repository into the networking tree.
Thanks,
Oliver
^ permalink raw reply
* Re: [PATCH 04/16] mm: allow PF_MEMALLOC from softirq context
From: Sebastian Andrzej Siewior @ 2012-06-26 16:55 UTC (permalink / raw)
To: Mel Gorman
Cc: Andrew Morton, Linux-MM, Linux-Netdev, LKML, David Miller,
Neil Brown, Peter Zijlstra, Mike Christie, Eric B Munson,
Eric Dumazet
In-Reply-To: <1340375443-22455-5-git-send-email-mgorman@suse.de>
On Fri, Jun 22, 2012 at 03:30:31PM +0100, Mel Gorman wrote:
> This is needed to allow network softirq packet processing to make
> use of PF_MEMALLOC.
> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> index b6c0727..5c6d9c6 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -2265,7 +2265,11 @@ gfp_to_alloc_flags(gfp_t gfp_mask)
> if (likely(!(gfp_mask & __GFP_NOMEMALLOC))) {
> if (gfp_mask & __GFP_MEMALLOC)
> alloc_flags |= ALLOC_NO_WATERMARKS;
> - else if (likely(!(gfp_mask & __GFP_NOMEMALLOC)) && !in_interrupt())
> + else if (in_serving_softirq() && (current->flags & PF_MEMALLOC))
> + alloc_flags |= ALLOC_NO_WATERMARKS;
> + else if (!in_interrupt() &&
> + ((current->flags & PF_MEMALLOC) ||
> + unlikely(test_thread_flag(TIF_MEMDIE))))
> alloc_flags |= ALLOC_NO_WATERMARKS;
> }
You allocate in RX path with __GFP_MEMALLOC and your sk->sk_allocation has
also __GFP_MEMALLOC set. That means you should get ALLOC_NO_WATERMARKS in
alloc_flags. Is this to done to avoid GFP annotations in skb_share_check() and
friends on your __netif_receive_skb() path?
Sebastian
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* Re: [net] ixgbe: Do not pad FCoE frames as this can cause issues with FCoE DDP
From: Greg KH @ 2012-06-26 16:33 UTC (permalink / raw)
To: Jeff Kirsher
Cc: David Miller, alexander.h.duyck, netdev, gospo, sassmann, stable
In-Reply-To: <1340696850.2255.17.camel@jtkirshe-mobl>
On Tue, Jun 26, 2012 at 12:47:30AM -0700, Jeff Kirsher wrote:
> On Tue, 2012-06-26 at 00:43 -0700, David Miller wrote:
> > You can't put things like "[3.4]" unquoted into the CC: list, that's
> > not kosher and vger rejected it.
>
> Sorry, that was what Greg told me to do. I did not realize it needed to
> be in quotes, my bad.
Greg told you no such thing, please use real email addresses on your cc:
lines, to not do so is crazy, and to blame me is sad.
greg k-h
^ permalink raw reply
* Re: New commands to configure IOV features
From: Alexander Duyck @ 2012-06-26 16:13 UTC (permalink / raw)
To: Yuval Mintz; +Cc: Ben Hutchings, Greg Rose, netdev@vger.kernel.org
In-Reply-To: <4FE9A963.7020602@broadcom.com>
On 06/26/2012 05:21 AM, Yuval Mintz wrote:
> On 05/07/2012 06:16 PM, Greg Rose wrote:
>
>> On Mon, 7 May 2012 14:17:54 +0300
>> Yuval Mintz <yuvalmin@broadcom.com> wrote:
>>
>>> I've tried to figure out if there was a standard interface
>>> (ethtool/iproute) through which a user could configure the number
>>> of vfs in his system.
>>>
>>> I've seen the RFC suggested in
>>> http://markmail.org/thread/qblfcv7zbxsxp7q6, and
>>> http://markmail.org/thread/fw54dcppmxuxoe6n, but failed to see any
>>> later references to it (commits or further discussion on this topic).
>>>
>>> How exactly are things standing with these RFCs? Were they abandoned?
>> The only way to configure the number of VFs continues to be through the
>> max_vfs module parameter. I've got a patch to do it through ethtool
>> sitting on the back burner but due to other requirements of my day job
>> I've not been able to work on it since last fall.
>>
>> - Greg
>
> Hi Ben,
>
> If I want to pick the RFCs and add support for configuring the number of
> VFs - do you think ethtool's the right place for such added support?
>
> I'm asking since as far as I can see, ethtool (today) doesn't contain any
> features related to virtual functions.
>
> Thanks,
> Yuval
I think the issue is that any class of PCI device could theoretically
support SR-IOV. For example there could be a storage controller out
there that supports spawning VFs, and an ethtool solution wouldn't work
for a device like that. Personally what I would like to see is a
solution that is more focused on the PCI side of the network adapters
instead of the network side when it comes to enabling VFs.
Thanks,
Alex
^ permalink raw reply
* Re: [net] ixgbe: Do not pad FCoE frames as this can cause issues with FCoE DDP
From: Ben Hutchings @ 2012-06-26 15:56 UTC (permalink / raw)
To: Alexander Duyck
Cc: jeffrey.t.kirsher, David Miller, netdev, gospo, sassmann, stable
In-Reply-To: <4FE9D9C2.5010209@intel.com>
[-- Attachment #1: Type: text/plain, Size: 1654 bytes --]
On Tue, 2012-06-26 at 08:48 -0700, Alexander Duyck wrote:
> On 06/26/2012 07:09 AM, Ben Hutchings wrote:
> > On Tue, 2012-06-26 at 00:53 -0700, Jeff Kirsher wrote:
> >> On Tue, 2012-06-26 at 00:50 -0700, David Miller wrote:
> >>> Sorry, quotes don't work either, what you did is still a SMTP syntax error,
> >>> here's what is in the bounce I get back:
> >>>
> >>> <stable@vger.kernel.org> "[3.4]",
> >>> Jeff Kirsher <jeffrey.t.kirsher@intel.com>
> >>> Illegal-Object: Syntax error in Cc: address found on vger.kernel.org:
> >>> Cc: <stable@vger.kernel.org>"[3.4]"
> >>> ^-missing end of address
> >> Grrr...
> >>
> >> I will re-send without the "[3.4]", Greg will just have to deal with it.
> > It's certainly not necessary to put anything like that in the real Cc
> > header. Many people put something like
> > 'Cc: <stable@vger.kernel.org> # 3.4' in the body; I don't know if that
> > is more like likely to result in the version being stripped when
> > generating mail recipients.
> >
> > Ben.
> >
> Couldn't 3.4 have been put in place of the recipient's name? So you do
> something like this:
> Cc: 3.4 <stable@vger.kernel.org>
Aside from the fact that recipient names with dots must be quoted in the
real header, it's relatively uncommon to use this format and the import
script I use for 3.2.y wouldn't notice it. (So I'd have to go back and
look at the original commit header to work out that it wasn't relevant.)
I don't know whether Greg's script extracts the recipient name.
Ben.
--
Ben Hutchings
Lowery's Law:
If it jams, force it. If it breaks, it needed replacing anyway.
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 828 bytes --]
^ permalink raw reply
* Re: [RFC net-next (v2) 12/14] ixgbe: set maximal number of default RSS queues
From: Alexander Duyck @ 2012-06-26 15:55 UTC (permalink / raw)
To: Yuval Mintz; +Cc: eilong, davem, netdev, Jeff Kirsher, John Fastabend
In-Reply-To: <4FE9983C.2060006@broadcom.com>
On 06/26/2012 04:08 AM, Yuval Mintz wrote:
>>> How about this:
>>> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_lib.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_lib.c
>>> index af1a531..23a8609 100644
>>> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_lib.c
>>> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_lib.c
>>> @@ -277,6 +277,8 @@ static inline bool ixgbe_set_rss_queues(struct ixgbe_adapter *adapter)
>>> bool ret = false;
>>> struct ixgbe_ring_feature *f = &adapter->ring_feature[RING_F_RSS];
>>>
>>> + f->indices = min_t(int, netif_get_num_default_rss_queues(), f->indices);
>>> +
>>> if (adapter->flags & IXGBE_FLAG_RSS_ENABLED) {
>>> f->mask = 0xF;
>>> adapter->num_rx_queues = f->indices;
>>> @@ -302,7 +304,9 @@ static inline bool ixgbe_set_fdir_queues(struct ixgbe_adapter *adapter)
>>> bool ret = false;
>>> struct ixgbe_ring_feature *f_fdir = &adapter->ring_feature[RING_F_FDIR];
>>>
>>> - f_fdir->indices = min_t(int, num_online_cpus(), f_fdir->indices);
>>> + f_fdir->indices = min_t(int, netif_get_num_default_rss_queues(),
>>> + f_fdir->indices);
>>> +
>>> f_fdir->mask = 0;
>>>
>>> /*
>>> @@ -339,8 +343,7 @@ static inline bool ixgbe_set_fcoe_queues(struct ixgbe_adapter *adapter)
>>> if (!(adapter->flags & IXGBE_FLAG_FCOE_ENABLED))
>>> return false;
>>>
>>> - f->indices = min_t(int, num_online_cpus(), f->indices);
>>> -
>>> + f->indices = min_t(int, f->indices, netif_get_num_default_rss_queues());
>>> adapter->num_rx_queues = 1;
>>> adapter->num_tx_queues = 1;
>>>
>> This makes much more sense, but still needs a few minor changes.
>
>
> Well, what about this one:
>
> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_lib.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_lib.c
> index af1a531..0dd1e51 100644
> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_lib.c
> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_lib.c
> @@ -277,6 +277,7 @@ static inline bool ixgbe_set_rss_queues(struct ixgbe_adapter *adapter)
> bool ret = false;
> struct ixgbe_ring_feature *f = &adapter->ring_feature[RING_F_RSS];
>
> + f->indices = min_t(int, netif_get_num_default_rss_queues(), f->indices);
> if (adapter->flags & IXGBE_FLAG_RSS_ENABLED) {
> f->mask = 0xF;
> adapter->num_rx_queues = f->indices;
> @@ -376,7 +377,7 @@ static inline bool ixgbe_set_dcb_queues(struct ixgbe_adapter *adapter)
>
> /* Map queue offset and counts onto allocated tx queues */
> per_tc_q = min_t(unsigned int, dev->num_tx_queues / tcs, DCB_QUEUE_CAP);
> - q = min_t(int, num_online_cpus(), per_tc_q);
> + q = min_t(int, netif_get_num_default_rss_queues(), per_tc_q);
>
> for (i = 0; i < tcs; i++) {
> netdev_set_tc_queue(dev, i, q, offset);
>
Add back in the bit for ixgbe_set_fcoe_queues and you should just about
have it in terms of limiting the number of RSS queues. That bit is
valid since the FCoE queues are based directly off of the RSS configuration.
One thing that just occurred to me though is that this is going to lock
the upper limit for us and we won't be able to override it if we
implement the set channels code. I believe the same thing goes for the
igb driver as well.
Is there any chance you could just bypass the ixgbe and igb drivers for
now and give us time to come up with a more complete solution that would
allow us to add the set_channels calls. One issue is I have a number of
ixgbe patches that are going to be completely rewriting this code anyway
so I could probably just add set_channels support and support for your
function once it is included in net-next.
Thanks,
Alex
^ permalink raw reply
* [PATCH 1/1] connector: use nlmsg_put() instead of NLMSG_PUT() macro.
From: Javier Martinez Canillas @ 2012-06-26 15:41 UTC (permalink / raw)
To: Evgeniy Polyakov; +Cc: netdev, Javier Martinez Canillas
The NLMSG_PUT() macro contains a hidden goto which makes the code hard
to audit and very error prone.
While been there also use the inline function nlmsg_data() instead of the
NLMSG_DATA() macro to do explicit type checking.
Signed-off-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk>
---
drivers/connector/connector.c | 12 ++++++------
1 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/connector/connector.c b/drivers/connector/connector.c
index a1f9364..a728d33 100644
--- a/drivers/connector/connector.c
+++ b/drivers/connector/connector.c
@@ -101,19 +101,19 @@ int cn_netlink_send(struct cn_msg *msg, u32 __group, gfp_t gfp_mask)
if (!skb)
return -ENOMEM;
- nlh = NLMSG_PUT(skb, 0, msg->seq, NLMSG_DONE, size - sizeof(*nlh));
+ nlh = nlmsg_put(skb, 0, msg->seq, NLMSG_DONE, size - sizeof(*nlh), 0);
+ if (!nlh) {
+ kfree_skb(skb);
+ return -EMSGSIZE;
+ }
- data = NLMSG_DATA(nlh);
+ data = nlmsg_data(nlh);
memcpy(data, msg, sizeof(*data) + msg->len);
NETLINK_CB(skb).dst_group = group;
return netlink_broadcast(dev->nls, skb, 0, group, gfp_mask);
-
-nlmsg_failure:
- kfree_skb(skb);
- return -EINVAL;
}
EXPORT_SYMBOL_GPL(cn_netlink_send);
--
1.7.7.6
^ permalink raw reply related
* [PATCH] bridge: Assign rtnl_link_ops to bridge devices created via ioctl (v2)
From: Stephen Hemminger @ 2012-06-26 15:48 UTC (permalink / raw)
To: davem; +Cc: Thomas Graf, netdev
In-Reply-To: <20120626151311.GB31808@canuck.infradead.org>
This ensures that bridges created with brctl(8) or ioctl(2) directly
also carry IFLA_LINKINFO when dumped over netlink. This also allows
to create a bridge with ioctl(2) and delete it with RTM_DELLINK.
Signed-off-by: Thomas Graf <tgraf@suug.ch>
Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>
---
v2 - Minor change to Thomas's original patch.
This is a bug fix, should go to stable as well.
net/bridge/br_if.c | 1 +
net/bridge/br_netlink.c | 2 +-
net/bridge/br_private.h | 1 +
3 files changed, 3 insertions(+), 1 deletion(-)
--- a/net/bridge/br_netlink.c 2012-06-26 08:07:18.459760113 -0700
+++ b/net/bridge/br_netlink.c 2012-06-26 08:30:16.729918150 -0700
@@ -208,7 +208,7 @@ static int br_validate(struct nlattr *tb
return 0;
}
-static struct rtnl_link_ops br_link_ops __read_mostly = {
+struct rtnl_link_ops br_link_ops __read_mostly = {
.kind = "bridge",
.priv_size = sizeof(struct net_bridge),
.setup = br_dev_setup,
--- a/net/bridge/br_private.h 2012-06-26 08:07:18.459760113 -0700
+++ b/net/bridge/br_private.h 2012-06-26 08:30:16.729918150 -0700
@@ -549,6 +549,7 @@ extern int (*br_fdb_test_addr_hook)(stru
#endif
/* br_netlink.c */
+extern struct rtnl_link_ops br_link_ops;
extern int br_netlink_init(void);
extern void br_netlink_fini(void);
extern void br_ifinfo_notify(int event, struct net_bridge_port *port);
--- a/net/bridge/br_if.c 2012-06-26 08:07:18.459760113 -0700
+++ b/net/bridge/br_if.c 2012-06-26 08:30:37.653708011 -0700
@@ -240,6 +240,7 @@ int br_add_bridge(struct net *net, const
return -ENOMEM;
dev_net_set(dev, net);
+ dev->rtnl_link_ops = &br_link_ops;
res = register_netdev(dev);
if (res)
^ permalink raw reply
* Re: [net] ixgbe: Do not pad FCoE frames as this can cause issues with FCoE DDP
From: Alexander Duyck @ 2012-06-26 15:48 UTC (permalink / raw)
To: Ben Hutchings
Cc: jeffrey.t.kirsher, David Miller, netdev, gospo, sassmann, stable
In-Reply-To: <1340719768.5330.2.camel@deadeye.wl.decadent.org.uk>
On 06/26/2012 07:09 AM, Ben Hutchings wrote:
> On Tue, 2012-06-26 at 00:53 -0700, Jeff Kirsher wrote:
>> On Tue, 2012-06-26 at 00:50 -0700, David Miller wrote:
>>> Sorry, quotes don't work either, what you did is still a SMTP syntax error,
>>> here's what is in the bounce I get back:
>>>
>>> <stable@vger.kernel.org> "[3.4]",
>>> Jeff Kirsher <jeffrey.t.kirsher@intel.com>
>>> Illegal-Object: Syntax error in Cc: address found on vger.kernel.org:
>>> Cc: <stable@vger.kernel.org>"[3.4]"
>>> ^-missing end of address
>> Grrr...
>>
>> I will re-send without the "[3.4]", Greg will just have to deal with it.
> It's certainly not necessary to put anything like that in the real Cc
> header. Many people put something like
> 'Cc: <stable@vger.kernel.org> # 3.4' in the body; I don't know if that
> is more like likely to result in the version being stripped when
> generating mail recipients.
>
> Ben.
>
Couldn't 3.4 have been put in place of the recipient's name? So you do
something like this:
Cc: 3.4 <stable@vger.kernel.org>
Thanks,
Alex
^ permalink raw reply
* [PATCH 1/1] atl1c: fix issue of transmit queue 0 timed out
From: Ren, Cloud @ 2012-06-26 15:33 UTC (permalink / raw)
To: davem, netdev, linux-kernel; +Cc: qca-linux-team, nic-devel, xiong
From: xiong <xiong@qca.qualcomm.com>
some people report atl1c could cause system hang with following
kernel trace info:
---------------------------------------
WARNING: at.../net/sched/sch_generic.c:258
dev_watchdog+0x1db/0x1d0()
...
NETDEV WATCHDOG: eth0 (atl1c): transmit queue 0 timed out
...
---------------------------------------
This is caused by netif_stop_queue calling when cable Link is down
but netif_wake_queue isn't called when cable Link is resume.
Signed-off-by: xiong <xiong@qca.qualcomm.com>
Signed-off-by: Cloud Ren <cjren@qca.qualcomm.com>
---
drivers/net/ethernet/atheros/atl1c/atl1c_main.c | 2 ++
1 files changed, 2 insertions(+), 0 deletions(-)
diff --git a/drivers/net/ethernet/atheros/atl1c/atl1c_main.c b/drivers/net/ethernet/atheros/atl1c/atl1c_main.c
index 85717cb..c2736c4 100644
--- a/drivers/net/ethernet/atheros/atl1c/atl1c_main.c
+++ b/drivers/net/ethernet/atheros/atl1c/atl1c_main.c
@@ -351,6 +351,8 @@ static void atl1c_common_task(struct work_struct *work)
atl1c_irq_disable(adapter);
atl1c_check_link_status(adapter);
atl1c_irq_enable(adapter);
+ if (netif_queue_stopped(netdev) && netif_carrier_ok(netdev))
+ netif_wake_queue(netdev);
}
}
--
1.7.7
^ permalink raw reply related
* Re: [PATCH 09/16] netvm: Allow skb allocation to use PFMEMALLOC reserves
From: Sebastian Andrzej Siewior @ 2012-06-26 15:27 UTC (permalink / raw)
To: Mel Gorman
Cc: Andrew Morton, Linux-MM, Linux-Netdev, LKML, David Miller,
Neil Brown, Peter Zijlstra, Mike Christie, Eric B Munson,
Eric Dumazet
In-Reply-To: <1340375443-22455-10-git-send-email-mgorman@suse.de>
On Fri, Jun 22, 2012 at 03:30:36PM +0100, Mel Gorman wrote:
> diff --git a/net/core/sock.c b/net/core/sock.c
> index 5c9ca2b..159dccc 100644
> --- a/net/core/sock.c
> +++ b/net/core/sock.c
> @@ -271,6 +271,9 @@ __u32 sysctl_rmem_default __read_mostly = SK_RMEM_MAX;
> int sysctl_optmem_max __read_mostly = sizeof(unsigned long)*(2*UIO_MAXIOV+512);
> EXPORT_SYMBOL(sysctl_optmem_max);
>
> +struct static_key memalloc_socks = STATIC_KEY_INIT_FALSE;
> +EXPORT_SYMBOL_GPL(memalloc_socks);
> +
This is used via sk_memalloc_socks() by SLAB.
>From 3da9ab9972845974da114c5a6624335e6371b2d5 Mon Sep 17 00:00:00 2001
From: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Date: Tue, 26 Jun 2012 17:18:20 +0200
Subject: [PATCH] export sk_memalloc_socks() only with CONFIG_NET
|mm/built-in.o: In function `atomic_read':
|include/asm/atomic.h:25: undefined reference to `memalloc_socks'
|include/asm/atomic.h:25: undefined reference to `memalloc_socks'
|include/asm/atomic.h:25: undefined reference to `memalloc_socks'
|include/asm/atomic.h:25: undefined reference to `memalloc_socks'
|include/asm/atomic.h:25: undefined reference to `memalloc_socks'
|mm/built-in.o:include/asm/atomic.h:25: more undefined references to `memalloc_socks' follow
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
include/net/sock.h | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/include/net/sock.h b/include/net/sock.h
index db0c20c..767c443 100644
--- a/include/net/sock.h
+++ b/include/net/sock.h
@@ -659,11 +659,20 @@ static inline bool sock_flag(const struct sock *sk, enum sock_flags flag)
return test_bit(flag, &sk->sk_flags);
}
+#ifdef CONFIG_NET
extern struct static_key memalloc_socks;
static inline int sk_memalloc_socks(void)
{
return static_key_false(&memalloc_socks);
}
+#else
+
+static inline int sk_memalloc_socks(void)
+{
+ return 0;
+}
+
+#endif
static inline gfp_t sk_gfp_atomic(struct sock *sk, gfp_t gfp_mask)
{
--
1.7.10
Sebastian
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply related
* Re: [PATCH] bridge: Assign rtnl_link_ops to bridge devices created via ioctl()
From: Thomas Graf @ 2012-06-26 15:13 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: davem, netdev
In-Reply-To: <20120626075940.099521a7@nehalam.linuxnetplumber.net>
On Tue, Jun 26, 2012 at 07:59:40AM -0700, Stephen Hemminger wrote:
> On Tue, 26 Jun 2012 10:56:15 +0200
> Thomas Graf <tgraf@suug.ch> wrote:
>
> > +void br_assign_rtnl_link_ops(struct net_device *dev)
> > +{
> > + dev->rtnl_link_ops = &br_link_ops;
> > +}
>
> I am fine with the concept, but maybe it would just be simpler to
> make br_link_ops public?
Either is fine with me. I chose the assignment function to keep the
struct read-only outside of br_netlink.c.
^ permalink raw reply
* Re: [PATCH] bridge: Assign rtnl_link_ops to bridge devices created via ioctl()
From: Stephen Hemminger @ 2012-06-26 14:59 UTC (permalink / raw)
To: Thomas Graf; +Cc: davem, netdev
In-Reply-To: <692d04316b78401b0256598155caae190d3f27c9.1340700973.git.tgraf@suug.ch>
On Tue, 26 Jun 2012 10:56:15 +0200
Thomas Graf <tgraf@suug.ch> wrote:
> +void br_assign_rtnl_link_ops(struct net_device *dev)
> +{
> + dev->rtnl_link_ops = &br_link_ops;
> +}
I am fine with the concept, but maybe it would just be simpler to
make br_link_ops public?
--- a/net/bridge/br_netlink.c 2012-06-22 08:27:50.837126940 -0700
+++ b/net/bridge/br_netlink.c 2012-06-26 07:56:33.510237340 -0700
@@ -208,7 +208,7 @@ static int br_validate(struct nlattr *tb
return 0;
}
-static struct rtnl_link_ops br_link_ops __read_mostly = {
+struct rtnl_link_ops br_link_ops __read_mostly = {
.kind = "bridge",
.priv_size = sizeof(struct net_bridge),
.setup = br_dev_setup,
--- a/net/bridge/br_private.h 2012-06-22 08:27:50.837126940 -0700
+++ b/net/bridge/br_private.h 2012-06-26 07:57:25.873711454 -0700
@@ -549,6 +549,7 @@ extern int (*br_fdb_test_addr_hook)(stru
#endif
/* br_netlink.c */
+extern struct rtnl_link_ops br_link_ops;
extern int br_netlink_init(void);
extern void br_netlink_fini(void);
extern void br_ifinfo_notify(int event, struct net_bridge_port *port);
--- a/net/bridge/br_if.c 2012-06-26 07:59:01.996746090 -0700
+++ b/net/bridge/br_if.c 2012-06-26 07:58:55.904807272 -0700
@@ -240,6 +240,7 @@ int br_add_bridge(struct net *net, const
return -ENOMEM;
dev_net_set(dev, net);
+ br->rtnl_link_ops = &br_link_ops;
res = register_netdev(dev);
if (res)
^ permalink raw reply
* Re: [PATCH 04/13] netfilter: regard users as refcount for l4proto's per-net data
From: Pablo Neira Ayuso @ 2012-06-26 14:47 UTC (permalink / raw)
To: Gao feng; +Cc: netdev, netfilter-devel
In-Reply-To: <4FE93375.1080803@cn.fujitsu.com>
On Tue, Jun 26, 2012 at 11:58:45AM +0800, Gao feng wrote:
> Hi Pablo:
> 于 2012年06月25日 19:20, Pablo Neira Ayuso 写道:
> > On Thu, Jun 21, 2012 at 10:36:41PM +0800, Gao feng wrote:
> >> Now, nf_proto_net's users is confusing.
> >> we should regard it as the refcount for l4proto's per-net data,
> >> because maybe there are two l4protos use the same per-net data.
> >>
> >> so increment pn->users when nf_conntrack_l4proto_register
> >> success, and decrement it for nf_conntrack_l4_unregister case.
> >>
> >> because nf_conntrack_l3proto_ipv[4|6] don't use the same per-net
> >> data,so we don't need to add a refcnt for their per-net data.
> >>
> >> Signed-off-by: Gao feng <gaofeng@cn.fujitsu.com>
> >> ---
> >> net/netfilter/nf_conntrack_proto.c | 76 ++++++++++++++++++++++--------------
> >> 1 files changed, 46 insertions(+), 30 deletions(-)
> >>
> >> diff --git a/net/netfilter/nf_conntrack_proto.c b/net/netfilter/nf_conntrack_proto.c
> >> index 9d6b6ab..63612e6 100644
> >> --- a/net/netfilter/nf_conntrack_proto.c
> >> +++ b/net/netfilter/nf_conntrack_proto.c
> > [...]
> >> @@ -458,23 +446,32 @@ int nf_conntrack_l4proto_register(struct net *net,
> >> struct nf_conntrack_l4proto *l4proto)
> >> {
> >> int ret = 0;
> >> + struct nf_proto_net *pn = NULL;
> >>
> >> if (l4proto->init_net) {
> >> ret = l4proto->init_net(net, l4proto->l3proto);
> >> if (ret < 0)
> >> - return ret;
> >> + goto out;
> >> }
> >>
> >> - ret = nf_ct_l4proto_register_sysctl(net, l4proto);
> >> + pn = nf_ct_l4proto_net(net, l4proto);
> >> + if (pn == NULL)
> >> + goto out;
> >
> > Same thing here, we're leaking memory allocated by l4proto->init_net.
>
> if pn is NULL,init_net can't allocate memory for pn->ctl_table.
> So I think it's not memory leak here.
Sorry, I meant to say the line below. But we've already clarified
this in patch 1/1.
> >> + ret = nf_ct_l4proto_register_sysctl(net, pn, l4proto);
> >> if (ret < 0)
> >> - return ret;
> >> + goto out;
> >>
> >> if (net == &init_net) {
> >> ret = nf_conntrack_l4proto_register_net(l4proto);
> >> - if (ret < 0)
> >> - nf_ct_l4proto_unregister_sysctl(net, l4proto);
> >> + if (ret < 0) {
> >> + nf_ct_l4proto_unregister_sysctl(net, pn, l4proto);
> >> + goto out;
> >
> > Better replace the two lines above by:
> >
> > goto out_register_net;
> >
> > and then...
> >
> >> + }
> >> }
> >>
> >> + pn->users++;
> >
> > out_register_net:
> > nf_ct_l4proto_unregister_sysctl(net, pn, l4proto);
> >
> >> +out:
> >> return ret;
> >
> > I think that this change is similar to patch 1/1, I think you should
> > send it as a separated patch.
> >
>
> Yes, It looks better.
> should I change this and rebase whole patchset or
> maybe you just apply this patchset and then I send a cleanup patch to do this?
This patch includes changes that are not included in the description,
so you have two choices:
1) You resend me this patch with appropriate description (including
the fact that you're fixing the same thing that patch 1/1 does). This
option still I don't like too much, since making two different things
in one single patch is nasty, but well if you push me...
2) you split the patch in two, with the appropriate descriptions each
and you'll make me happy.
--
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
* Re: [net-next patch] bnx2x: Change bnx2x_tests_str_arr to static char
From: Joe Perches @ 2012-06-26 14:41 UTC (permalink / raw)
To: Merav Sicron; +Cc: davem, netdev, eilong
In-Reply-To: <1340727063-23870-1-git-send-email-meravs@broadcom.com>
On Tue, 2012-06-26 at 19:11 +0300, Merav Sicron wrote:
> This patch changes the definition of bnx2x_tests_str_arr from char to static
> char.
[]
> diff --git a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_ethtool.c b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_ethtool.c
[]
> @@ -1600,7 +1600,7 @@ static int bnx2x_set_pauseparam(struct net_device *dev,
> return 0;
> }
>
> -char *bnx2x_tests_str_arr[BNX2X_NUM_TESTS_SF] = {
> +static char *bnx2x_tests_str_arr[BNX2X_NUM_TESTS_SF] = {
> "register_test (offline) ",
> "memory_test (offline) ",
> "int_loopback_test (offline)",
Should probably be const too.
^ permalink raw reply
* Re: [PATCH 01/13] netfilter: fix problem with proto register
From: Pablo Neira Ayuso @ 2012-06-26 14:36 UTC (permalink / raw)
To: Gao feng; +Cc: netdev, netfilter-devel
In-Reply-To: <4FE92F1E.9020901@cn.fujitsu.com>
On Tue, Jun 26, 2012 at 11:40:14AM +0800, Gao feng wrote:
> Hi Pablo:
>
> 于 2012年06月25日 19:12, Pablo Neira Ayuso 写道:
> > On Thu, Jun 21, 2012 at 10:36:38PM +0800, Gao feng wrote:
> >> before commit 2c352f444ccfa966a1aa4fd8e9ee29381c467448
> >> (netfilter: nf_conntrack: prepare namespace support for
> >> l4 protocol trackers), we register sysctl before register
> >> protos, so if sysctl is registered faild, the protos will
> >> not be registered.
> >>
> >> but now, we register protos first, and when register
> >> sysctl failed, we can use protos too, it's different
> >> from before.
> >
> > No, this has to be an all-or-nothing game. If one fails, everything
> > else that you've registered has to be unregistered.
>
> indeed,this is an all-or-nothing game right now,please look at the ipv4_net_init,
> when we register nf_conntrack_l3proto_ipv4 failed,we will unregister the already
> registered l4protoes, and in nf_conntrack_l4proto_unregister,we will call
> nf_ct_l4proto_unregister_sysctl to free the sysctl table.
I see proto->init_net allocates in->ctl_table, then
nf_ct_l3proto_register_sysctl release it if it fails. I got confused
because I did not see where that memory was being freed. Then, it's
good.
Still one more thing:
> >> so change to register sysctl before register protos.
> >>
> >> Signed-off-by: Gao feng <gaofeng@cn.fujitsu.com>
> >> ---
> >> net/netfilter/nf_conntrack_proto.c | 36 +++++++++++++++++++++++-------------
> >> 1 files changed, 23 insertions(+), 13 deletions(-)
> >>
> >> diff --git a/net/netfilter/nf_conntrack_proto.c b/net/netfilter/nf_conntrack_proto.c
> >> index 1ea9194..9bd88aa 100644
> >> --- a/net/netfilter/nf_conntrack_proto.c
> >> +++ b/net/netfilter/nf_conntrack_proto.c
> >> @@ -253,18 +253,23 @@ int nf_conntrack_l3proto_register(struct net *net,
> >> {
> >> int ret = 0;
> >>
> >> - if (net == &init_net)
> >> - ret = nf_conntrack_l3proto_register_net(proto);
> >> + if (proto->init_net) {
I think proto->init_net has to be mandatory since all protocol support
pernet already. We can add BUG_ON at the beginning of the function if
proto->init_net is not defined.
I can manually add that to the patch if you see no inconvenience with
it.
--
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
* Re: [net] ixgbe: Do not pad FCoE frames as this can cause issues with FCoE DDP
From: Ben Hutchings @ 2012-06-26 14:09 UTC (permalink / raw)
To: jeffrey.t.kirsher
Cc: David Miller, alexander.h.duyck, netdev, gospo, sassmann, stable
In-Reply-To: <1340697238.2255.20.camel@jtkirshe-mobl>
[-- Attachment #1: Type: text/plain, Size: 987 bytes --]
On Tue, 2012-06-26 at 00:53 -0700, Jeff Kirsher wrote:
> On Tue, 2012-06-26 at 00:50 -0700, David Miller wrote:
> > Sorry, quotes don't work either, what you did is still a SMTP syntax error,
> > here's what is in the bounce I get back:
> >
> > <stable@vger.kernel.org> "[3.4]",
> > Jeff Kirsher <jeffrey.t.kirsher@intel.com>
> > Illegal-Object: Syntax error in Cc: address found on vger.kernel.org:
> > Cc: <stable@vger.kernel.org>"[3.4]"
> > ^-missing end of address
>
> Grrr...
>
> I will re-send without the "[3.4]", Greg will just have to deal with it.
It's certainly not necessary to put anything like that in the real Cc
header. Many people put something like
'Cc: <stable@vger.kernel.org> # 3.4' in the body; I don't know if that
is more like likely to result in the version being stripped when
generating mail recipients.
Ben.
--
Ben Hutchings
Lowery's Law:
If it jams, force it. If it breaks, it needed replacing anyway.
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 828 bytes --]
^ permalink raw reply
* [net-next patch] bnx2x: Change bnx2x_tests_str_arr to static char
From: Merav Sicron @ 2012-06-26 16:11 UTC (permalink / raw)
To: davem, netdev, eilong; +Cc: Merav Sicron
This patch changes the definition of bnx2x_tests_str_arr from char to static
char. This correction will also eliminate the sparse warning created in commit
cf2c1df62e065bfc15e38daf2d3479a56b320f29.
Signed-off-by: Merav Sicron <meravs@broadcom.com>
Signed-off-by: Eilon Greenstein <eilong@broadcom.com>
---
Hi Dave,
Please consider applying this patch to net-next.
Thanks,
Merav
drivers/net/ethernet/broadcom/bnx2x/bnx2x_ethtool.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_ethtool.c b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_ethtool.c
index 1f8c156..bff3129 100644
--- a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_ethtool.c
+++ b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_ethtool.c
@@ -1600,7 +1600,7 @@ static int bnx2x_set_pauseparam(struct net_device *dev,
return 0;
}
-char *bnx2x_tests_str_arr[BNX2X_NUM_TESTS_SF] = {
+static char *bnx2x_tests_str_arr[BNX2X_NUM_TESTS_SF] = {
"register_test (offline) ",
"memory_test (offline) ",
"int_loopback_test (offline)",
--
1.7.10
^ permalink raw reply related
* Re: New commands to configure IOV features
From: Yuval Mintz @ 2012-06-26 12:21 UTC (permalink / raw)
To: Ben Hutchings; +Cc: Greg Rose, netdev@vger.kernel.org
In-Reply-To: <20120507081634.000003f8.gregory.v.rose@intel.com>
On 05/07/2012 06:16 PM, Greg Rose wrote:
> On Mon, 7 May 2012 14:17:54 +0300
> Yuval Mintz <yuvalmin@broadcom.com> wrote:
>
>> I've tried to figure out if there was a standard interface
>> (ethtool/iproute) through which a user could configure the number
>> of vfs in his system.
>>
>> I've seen the RFC suggested in
>> http://markmail.org/thread/qblfcv7zbxsxp7q6, and
>> http://markmail.org/thread/fw54dcppmxuxoe6n, but failed to see any
>> later references to it (commits or further discussion on this topic).
>>
>> How exactly are things standing with these RFCs? Were they abandoned?
>
> The only way to configure the number of VFs continues to be through the
> max_vfs module parameter. I've got a patch to do it through ethtool
> sitting on the back burner but due to other requirements of my day job
> I've not been able to work on it since last fall.
>
> - Greg
Hi Ben,
If I want to pick the RFCs and add support for configuring the number of
VFs - do you think ethtool's the right place for such added support?
I'm asking since as far as I can see, ethtool (today) doesn't contain any
features related to virtual functions.
Thanks,
Yuval
^ permalink raw reply
* Re: [PATCH 3/3] net: fec: add phy-reset-interval for device tree probe
From: Shawn Guo @ 2012-06-26 12:13 UTC (permalink / raw)
To: Florian Fainelli; +Cc: David S. Miller, netdev, linux-arm-kernel
In-Reply-To: <2921547.BkALd1Y1Bk@flexo>
On Tue, Jun 26, 2012 at 01:55:05PM +0200, Florian Fainelli wrote:
> > + of_property_read_u32(np, "phy-reset-interval", &msec);
> > phy_reset = of_get_named_gpio(np, "phy-reset-gpios", 0);
> > err = gpio_request_one(phy_reset, GPIOF_OUT_INIT_LOW, "phy-reset");
> > if (err) {
> > pr_debug("FEC: failed to get gpio phy-reset: %d\n", err);
> > return;
> > }
> > - msleep(1);
> > + msleep(msec);
>
> You might want to check the value of the property here to make sure it is in
> the msleep() acceptable range.
>
Sounds good. I think a sensible reset duration should be less one
second, so let's force 1000 ms to be the possible max one?
--
Regards,
Shawn
^ permalink raw reply
* Re: [PATCH 3/3] net: fec: add phy-reset-interval for device tree probe
From: Florian Fainelli @ 2012-06-26 11:55 UTC (permalink / raw)
To: Shawn Guo; +Cc: David S. Miller, netdev, linux-arm-kernel
In-Reply-To: <1340700308-8315-4-git-send-email-shawn.guo@linaro.org>
Hi,
On Tuesday 26 June 2012 16:45:08 Shawn Guo wrote:
> Different boards may require different phy reset interval time. Add
> property phy-reset-interval for device tree probe, so that the boards
> that need a longer interval time can specify it in their device tree.
>
> Along with the update to phy related stuff, it also makes a minor fix
> on phy-reset-gpios in binding document to have it be optional to match
> the driver code.
>
> Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
> ---
> Documentation/devicetree/bindings/net/fsl-fec.txt | 5 ++++-
> drivers/net/ethernet/freescale/fec.c | 4 +++-
> 2 files changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/net/fsl-fec.txt
b/Documentation/devicetree/bindings/net/fsl-fec.txt
> index 7ab9e1a..74a4ce7 100644
> --- a/Documentation/devicetree/bindings/net/fsl-fec.txt
> +++ b/Documentation/devicetree/bindings/net/fsl-fec.txt
> @@ -7,10 +7,13 @@ Required properties:
> - phy-mode : String, operation mode of the PHY interface.
> Supported values are: "mii", "gmii", "sgmii", "tbi", "rmii",
> "rgmii", "rgmii-id", "rgmii-rxid", "rgmii-txid", "rtbi", "smii".
> -- phy-reset-gpios : Should specify the gpio for phy reset
>
> Optional properties:
> - local-mac-address : 6 bytes, mac address
> +- phy-reset-gpios : Should specify the gpio for phy reset
> +- phy-reset-interval : Reset interval time in milliseconds. Should present
> + only if property "phy-reset-gpios" is available. When "phy-reset-gpios"
> + is available, missing the property will have the interval be 1
millisecond.
>
> Example:
>
> diff --git a/drivers/net/ethernet/freescale/fec.c
b/drivers/net/ethernet/freescale/fec.c
> index 4dce9e3..86ecaae 100644
> --- a/drivers/net/ethernet/freescale/fec.c
> +++ b/drivers/net/ethernet/freescale/fec.c
> @@ -1507,18 +1507,20 @@ static int __devinit fec_get_phy_mode_dt(struct
platform_device *pdev)
> static void __devinit fec_reset_phy(struct platform_device *pdev)
> {
> int err, phy_reset;
> + int msec = 1;
> struct device_node *np = pdev->dev.of_node;
>
> if (!np)
> return;
>
> + of_property_read_u32(np, "phy-reset-interval", &msec);
> phy_reset = of_get_named_gpio(np, "phy-reset-gpios", 0);
> err = gpio_request_one(phy_reset, GPIOF_OUT_INIT_LOW, "phy-reset");
> if (err) {
> pr_debug("FEC: failed to get gpio phy-reset: %d\n", err);
> return;
> }
> - msleep(1);
> + msleep(msec);
You might want to check the value of the property here to make sure it is in
the msleep() acceptable range.
> gpio_set_value(phy_reset, 1);
> }
> #else /* CONFIG_OF */
> --
> 1.7.5.4
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [net-next RFC V3 PATCH 4/6] tuntap: multiqueue support
From: Michael S. Tsirkin @ 2012-06-26 11:54 UTC (permalink / raw)
To: Jason Wang
Cc: habanero, netdev, linux-kernel, krkumar2, tahm, akong, davem,
shemminger, mashirle, Eric Dumazet
In-Reply-To: <4FE94E39.6070305@redhat.com>
On Tue, Jun 26, 2012 at 01:52:57PM +0800, Jason Wang wrote:
> On 06/25/2012 04:25 PM, Michael S. Tsirkin wrote:
> >On Mon, Jun 25, 2012 at 02:10:18PM +0800, Jason Wang wrote:
> >>This patch adds multiqueue support for tap device. This is done by abstracting
> >>each queue as a file/socket and allowing multiple sockets to be attached to the
> >>tuntap device (an array of tun_file were stored in the tun_struct). Userspace
> >>could write and read from those files to do the parallel packet
> >>sending/receiving.
> >>
> >>Unlike the previous single queue implementation, the socket and device were
> >>loosely coupled, each of them were allowed to go away first. In order to let the
> >>tx path lockless, netif_tx_loch_bh() is replaced by RCU/NETIF_F_LLTX to
> >>synchronize between data path and system call.
> >Don't use LLTX/RCU. It's not worth it.
> >Use something like netif_set_real_num_tx_queues.
> >
>
> For LLTX, maybe it's better to convert it to alloc_netdev_mq() to
> let the kernel see all queues and make the queue stopping and
> per-queue stats eaiser.
> RCU is used to handle the attaching/detaching when tun/tap is
> sending and receiving packets which looks reasonalbe for me.
Yes but do we have to allow this? How about we always ask
userspace to attach to all active queues?
> Not
> sure netif_set_real_num_tx_queues() can help in this situation.
Check it out.
> >>The tx queue selecting is first based on the recorded rxq index of an skb, it
> >>there's no such one, then choosing based on rx hashing (skb_get_rxhash()).
> >>
> >>Signed-off-by: Jason Wang<jasowang@redhat.com>
> >Interestingly macvtap switched to hashing first:
> >ef0002b577b52941fb147128f30bd1ecfdd3ff6d
> >(the commit log is corrupted but see what it
> >does in the patch).
> >Any idea why?
> >
> >>---
> >> drivers/net/tun.c | 371 +++++++++++++++++++++++++++++++++--------------------
> >> 1 files changed, 232 insertions(+), 139 deletions(-)
> >>
> >>diff --git a/drivers/net/tun.c b/drivers/net/tun.c
> >>index 8233b0a..5c26757 100644
> >>--- a/drivers/net/tun.c
> >>+++ b/drivers/net/tun.c
> >>@@ -107,6 +107,8 @@ struct tap_filter {
> >> unsigned char addr[FLT_EXACT_COUNT][ETH_ALEN];
> >> };
> >>
> >>+#define MAX_TAP_QUEUES (NR_CPUS< 16 ? NR_CPUS : 16)
> >Why the limit? I am guessing you copied this from macvtap?
> >This is problematic for a number of reasons:
> > - will not play well with migration
> > - will not work well for a large guest
> >
> >Yes, macvtap needs to be fixed too.
> >
> >I am guessing what it is trying to prevent is queueing
> >up a huge number of packets?
> >So just divide the default tx queue limit by the # of queues.
>
> Not sure,
> another reasons I can guess:
> - to prevent storing a large array of pointers in tun_struct or macvlan_dev.
OK so with the limit of e.g. 1024 we'd allocate at most
2 pages of memory. This doesn't look too bad. 1024 is probably a
high enough limit: modern hypervisors seem to support on the order
of 100-200 CPUs so this leaves us some breathing space
if we want to match a queue per guest CPU.
Of course we need to limit the packets per queue
in such a setup more aggressively. 1000 packets * 1000 queues
* 64K per packet is too much.
> - it may not be suitable to allow the number of virtqueues greater
> than the number of physical queues in the card
Maybe for macvtap, here we have no idea which card we
are working with and how many queues it has.
> >
> >And by the way, for MQ applications maybe we can finally
> >ignore tx queue altogether and limit the total number
> >of bytes queued?
> >To avoid regressions we can make it large like 64M/# queues.
> >Could be a separate patch I think, and for a single queue
> >might need a compatible mode though I am not sure.
>
> Could you explain more about this?
> Did you mean to have a total
> sndbuf for all sockets that attached to tun/tap?
Consider that we currently limit the # of
packets queued at tun for xmit to userspace.
Some limit is needed but # of packets sounds
very silly - limiting the total memory
might be more reasonable.
In case of multiqueue, we really care about
total # of packets or total memory, but a simple
approximation could be to divide the allocation
between active queues equally.
qdisc also queues some packets, that logic is
using # of packets anyway. So either make that
1000/# queues, or even set to 0 as Eric once
suggested.
> >>+
> >> struct tun_file {
> >> struct sock sk;
> >> struct socket socket;
> >>@@ -114,16 +116,18 @@ struct tun_file {
> >> int vnet_hdr_sz;
> >> struct tap_filter txflt;
> >> atomic_t count;
> >>- struct tun_struct *tun;
> >>+ struct tun_struct __rcu *tun;
> >> struct net *net;
> >> struct fasync_struct *fasync;
> >> unsigned int flags;
> >>+ u16 queue_index;
> >> };
> >>
> >> struct tun_sock;
> >>
> >> struct tun_struct {
> >>- struct tun_file *tfile;
> >>+ struct tun_file *tfiles[MAX_TAP_QUEUES];
> >>+ unsigned int numqueues;
> >> unsigned int flags;
> >> uid_t owner;
> >> gid_t group;
> >>@@ -138,80 +142,159 @@ struct tun_struct {
> >> #endif
> >> };
> >>
> >>-static int tun_attach(struct tun_struct *tun, struct file *file)
> >>+static DEFINE_SPINLOCK(tun_lock);
> >>+
> >>+/*
> >>+ * tun_get_queue(): calculate the queue index
> >>+ * - if skbs comes from mq nics, we can just borrow
> >>+ * - if not, calculate from the hash
> >>+ */
> >>+static struct tun_file *tun_get_queue(struct net_device *dev,
> >>+ struct sk_buff *skb)
> >> {
> >>- struct tun_file *tfile = file->private_data;
> >>- int err;
> >>+ struct tun_struct *tun = netdev_priv(dev);
> >>+ struct tun_file *tfile = NULL;
> >>+ int numqueues = tun->numqueues;
> >>+ __u32 rxq;
> >>
> >>- ASSERT_RTNL();
> >>+ BUG_ON(!rcu_read_lock_held());
> >>
> >>- netif_tx_lock_bh(tun->dev);
> >>+ if (!numqueues)
> >>+ goto out;
> >>
> >>- err = -EINVAL;
> >>- if (tfile->tun)
> >>+ if (numqueues == 1) {
> >>+ tfile = rcu_dereference(tun->tfiles[0]);
> >Instead of hacks like this, you can ask for an MQ
> >flag to be set in SETIFF. Then you won't need to
> >handle attach/detach at random times.
>
> Consier user switch between a sq guest to mq guest, qemu would
> attach or detach the fd which could not be expceted in kernel.
Can't userspace keep it attached always, just deactivate MQ?
> >And most of the scary num_queues checks can go away.
>
> Even we has a MQ flag, userspace could still just attach one queue
> to the device.
I think we allow too much flexibility if we let
userspace detach a random queue.
Maybe only allow attaching/detaching with MQ off?
If userspace wants to attach/detach, clear MQ first?
Alternatively, attach/detach all queues in one ioctl?
> >You can then also ask userspace about the max # of queues
> >to expect if you want to save some memory.
> >
>
> Yes, good suggestion.
> >> goto out;
> >>+ }
> >>
> >>- err = -EBUSY;
> >>- if (tun->tfile)
> >>+ if (likely(skb_rx_queue_recorded(skb))) {
> >>+ rxq = skb_get_rx_queue(skb);
> >>+
> >>+ while (unlikely(rxq>= numqueues))
> >>+ rxq -= numqueues;
> >>+
> >>+ tfile = rcu_dereference(tun->tfiles[rxq]);
> >> goto out;
> >>+ }
> >>
> >>- err = 0;
> >>- tfile->tun = tun;
> >>- tun->tfile = tfile;
> >>- netif_carrier_on(tun->dev);
> >>- dev_hold(tun->dev);
> >>- sock_hold(&tfile->sk);
> >>- atomic_inc(&tfile->count);
> >>+ /* Check if we can use flow to select a queue */
> >>+ rxq = skb_get_rxhash(skb);
> >>+ if (rxq) {
> >>+ u32 idx = ((u64)rxq * numqueues)>> 32;
> >This completely confuses me. What's the logic here?
> >How do we even know it's in range?
> >
>
> rxq is a u32, so the result should be less than numqueues.
Aha. So the point is to use multiply+shift instead of %?
Please add a comment.
> >>+ tfile = rcu_dereference(tun->tfiles[idx]);
> >>+ goto out;
> >>+ }
> >>
> >>+ tfile = rcu_dereference(tun->tfiles[0]);
> >> out:
> >>- netif_tx_unlock_bh(tun->dev);
> >>- return err;
> >>+ return tfile;
> >> }
> >>
> >>-static void __tun_detach(struct tun_struct *tun)
> >>+static int tun_detach(struct tun_file *tfile, bool clean)
> >> {
> >>- struct tun_file *tfile = tun->tfile;
> >>- /* Detach from net device */
> >>- netif_tx_lock_bh(tun->dev);
> >>- netif_carrier_off(tun->dev);
> >>- tun->tfile = NULL;
> >>- netif_tx_unlock_bh(tun->dev);
> >>-
> >>- /* Drop read queue */
> >>- skb_queue_purge(&tfile->socket.sk->sk_receive_queue);
> >>-
> >>- /* Drop the extra count on the net device */
> >>- dev_put(tun->dev);
> >>-}
> >>+ struct tun_struct *tun;
> >>+ struct net_device *dev = NULL;
> >>+ bool destroy = false;
> >>
> >>-static void tun_detach(struct tun_struct *tun)
> >>-{
> >>- rtnl_lock();
> >>- __tun_detach(tun);
> >>- rtnl_unlock();
> >>-}
> >>+ spin_lock(&tun_lock);
> >>
> >>-static struct tun_struct *__tun_get(struct tun_file *tfile)
> >>-{
> >>- struct tun_struct *tun = NULL;
> >>+ tun = rcu_dereference_protected(tfile->tun,
> >>+ lockdep_is_held(&tun_lock));
> >>+ if (tun) {
> >>+ u16 index = tfile->queue_index;
> >>+ BUG_ON(index>= tun->numqueues);
> >>+ dev = tun->dev;
> >>+
> >>+ rcu_assign_pointer(tun->tfiles[index],
> >>+ tun->tfiles[tun->numqueues - 1]);
> >>+ tun->tfiles[index]->queue_index = index;
> >>+ rcu_assign_pointer(tfile->tun, NULL);
> >>+ --tun->numqueues;
> >>+ sock_put(&tfile->sk);
> >>
> >>- if (atomic_inc_not_zero(&tfile->count))
> >>- tun = tfile->tun;
> >>+ if (tun->numqueues == 0&& !(tun->flags& TUN_PERSIST))
> >>+ destroy = true;
> >Please don't use flags like that. Use dedicated labels and goto there on error.
>
> ok.
> >
> >>+ }
> >>
> >>- return tun;
> >>+ spin_unlock(&tun_lock);
> >>+
> >>+ synchronize_rcu();
> >>+ if (clean)
> >>+ sock_put(&tfile->sk);
> >>+
> >>+ if (destroy) {
> >>+ rtnl_lock();
> >>+ if (dev->reg_state == NETREG_REGISTERED)
> >>+ unregister_netdevice(dev);
> >>+ rtnl_unlock();
> >>+ }
> >>+
> >>+ return 0;
> >> }
> >>
> >>-static struct tun_struct *tun_get(struct file *file)
> >>+static void tun_detach_all(struct net_device *dev)
> >> {
> >>- return __tun_get(file->private_data);
> >>+ struct tun_struct *tun = netdev_priv(dev);
> >>+ struct tun_file *tfile, *tfile_list[MAX_TAP_QUEUES];
> >>+ int i, j = 0;
> >>+
> >>+ spin_lock(&tun_lock);
> >>+
> >>+ for (i = 0; i< MAX_TAP_QUEUES&& tun->numqueues; i++) {
> >>+ tfile = rcu_dereference_protected(tun->tfiles[i],
> >>+ lockdep_is_held(&tun_lock));
> >>+ BUG_ON(!tfile);
> >>+ wake_up_all(&tfile->wq.wait);
> >>+ tfile_list[j++] = tfile;
> >>+ rcu_assign_pointer(tfile->tun, NULL);
> >>+ --tun->numqueues;
> >>+ }
> >>+ BUG_ON(tun->numqueues != 0);
> >>+ /* guarantee that any future tun_attach will fail */
> >>+ tun->numqueues = MAX_TAP_QUEUES;
> >>+ spin_unlock(&tun_lock);
> >>+
> >>+ synchronize_rcu();
> >>+ for (--j; j>= 0; j--)
> >>+ sock_put(&tfile_list[j]->sk);
> >> }
> >>
> >>-static void tun_put(struct tun_struct *tun)
> >>+static int tun_attach(struct tun_struct *tun, struct file *file)
> >> {
> >>- struct tun_file *tfile = tun->tfile;
> >>+ struct tun_file *tfile = file->private_data;
> >>+ int err;
> >>+
> >>+ ASSERT_RTNL();
> >>+
> >>+ spin_lock(&tun_lock);
> >>
> >>- if (atomic_dec_and_test(&tfile->count))
> >>- tun_detach(tfile->tun);
> >>+ err = -EINVAL;
> >>+ if (rcu_dereference_protected(tfile->tun, lockdep_is_held(&tun_lock)))
> >>+ goto out;
> >>+
> >>+ err = -EBUSY;
> >>+ if (!(tun->flags& TUN_TAP_MQ)&& tun->numqueues == 1)
> >>+ goto out;
> >>+
> >>+ if (tun->numqueues == MAX_TAP_QUEUES)
> >>+ goto out;
> >>+
> >>+ err = 0;
> >>+ tfile->queue_index = tun->numqueues;
> >>+ rcu_assign_pointer(tfile->tun, tun);
> >>+ rcu_assign_pointer(tun->tfiles[tun->numqueues], tfile);
> >>+ sock_hold(&tfile->sk);
> >>+ tun->numqueues++;
> >>+
> >>+ if (tun->numqueues == 1)
> >>+ netif_carrier_on(tun->dev);
> >>+
> >>+ /* device is allowed to go away first, so no need to hold extra
> >>+ * refcnt. */
> >>+
> >>+out:
> >>+ spin_unlock(&tun_lock);
> >>+ return err;
> >> }
> >>
> >> /* TAP filtering */
> >>@@ -331,16 +414,7 @@ static const struct ethtool_ops tun_ethtool_ops;
> >> /* Net device detach from fd. */
> >> static void tun_net_uninit(struct net_device *dev)
> >> {
> >>- struct tun_struct *tun = netdev_priv(dev);
> >>- struct tun_file *tfile = tun->tfile;
> >>-
> >>- /* Inform the methods they need to stop using the dev.
> >>- */
> >>- if (tfile) {
> >>- wake_up_all(&tfile->wq.wait);
> >>- if (atomic_dec_and_test(&tfile->count))
> >>- __tun_detach(tun);
> >>- }
> >>+ tun_detach_all(dev);
> >> }
> >>
> >> /* Net device open. */
> >>@@ -360,10 +434,10 @@ static int tun_net_close(struct net_device *dev)
> >> /* Net device start xmit */
> >> static netdev_tx_t tun_net_xmit(struct sk_buff *skb, struct net_device *dev)
> >> {
> >>- struct tun_struct *tun = netdev_priv(dev);
> >>- struct tun_file *tfile = tun->tfile;
> >>+ struct tun_file *tfile = NULL;
> >>
> >>- tun_debug(KERN_INFO, tun, "tun_net_xmit %d\n", skb->len);
> >>+ rcu_read_lock();
> >>+ tfile = tun_get_queue(dev, skb);
> >>
> >> /* Drop packet if interface is not attached */
> >> if (!tfile)
> >>@@ -381,7 +455,8 @@ static netdev_tx_t tun_net_xmit(struct sk_buff *skb, struct net_device *dev)
> >>
> >> if (skb_queue_len(&tfile->socket.sk->sk_receive_queue)
> >> >= dev->tx_queue_len) {
> >>- if (!(tun->flags& TUN_ONE_QUEUE)) {
> >>+ if (!(tfile->flags& TUN_ONE_QUEUE)&&
> >Which patch moved flags from tun to tfile?
>
> Patch 1 cache the tun->flags in tfile, but it seems this may let the
> flags out of sync. So we'd better to use the one in tun_struct.
> >
> >>+ !(tfile->flags& TUN_TAP_MQ)) {
> >> /* Normal queueing mode. */
> >> /* Packet scheduler handles dropping of further packets. */
> >> netif_stop_queue(dev);
> >>@@ -390,7 +465,7 @@ static netdev_tx_t tun_net_xmit(struct sk_buff *skb, struct net_device *dev)
> >> * error is more appropriate. */
> >> dev->stats.tx_fifo_errors++;
> >> } else {
> >>- /* Single queue mode.
> >>+ /* Single queue mode or multi queue mode.
> >> * Driver handles dropping of all packets itself. */
> >Please don't do this. Stop the queue on overrun as appropriate.
> >ONE_QUEUE is a legacy hack.
> >
> >BTW we really should stop queue before we start dropping packets,
> >but that can be a separate patch.
>
> The problem here is the using of NETIF_F_LLTX. Kernel could only see
> one queue even for a multiqueue tun/tap. If we use
> netif_stop_queue(), all other queues would be stopped also.
Another reason not to use LLTX?
> >> goto drop;
> >> }
> >>@@ -408,9 +483,11 @@ static netdev_tx_t tun_net_xmit(struct sk_buff *skb, struct net_device *dev)
> >> kill_fasync(&tfile->fasync, SIGIO, POLL_IN);
> >> wake_up_interruptible_poll(&tfile->wq.wait, POLLIN |
> >> POLLRDNORM | POLLRDBAND);
> >>+ rcu_read_unlock();
> >> return NETDEV_TX_OK;
> >>
> >> drop:
> >>+ rcu_read_unlock();
> >> dev->stats.tx_dropped++;
> >> kfree_skb(skb);
> >> return NETDEV_TX_OK;
> >>@@ -527,16 +604,22 @@ static void tun_net_init(struct net_device *dev)
> >> static unsigned int tun_chr_poll(struct file *file, poll_table * wait)
> >> {
> >> struct tun_file *tfile = file->private_data;
> >>- struct tun_struct *tun = __tun_get(tfile);
> >>+ struct tun_struct *tun = NULL;
> >> struct sock *sk;
> >> unsigned int mask = 0;
> >>
> >>- if (!tun)
> >>+ if (!tfile)
> >> return POLLERR;
> >>
> >>- sk = tfile->socket.sk;
> >>+ rcu_read_lock();
> >>+ tun = rcu_dereference(tfile->tun);
> >>+ if (!tun) {
> >>+ rcu_read_unlock();
> >>+ return POLLERR;
> >>+ }
> >>+ rcu_read_unlock();
> >>
> >>- tun_debug(KERN_INFO, tun, "tun_chr_poll\n");
> >>+ sk =&tfile->sk;
> >>
> >> poll_wait(file,&tfile->wq.wait, wait);
> >>
> >>@@ -548,10 +631,12 @@ static unsigned int tun_chr_poll(struct file *file, poll_table * wait)
> >> sock_writeable(sk)))
> >> mask |= POLLOUT | POLLWRNORM;
> >>
> >>- if (tun->dev->reg_state != NETREG_REGISTERED)
> >>+ rcu_read_lock();
> >>+ tun = rcu_dereference(tfile->tun);
> >>+ if (!tun || tun->dev->reg_state != NETREG_REGISTERED)
> >> mask = POLLERR;
> >>+ rcu_read_unlock();
> >>
> >>- tun_put(tun);
> >> return mask;
> >> }
> >>
> >>@@ -708,9 +793,12 @@ static ssize_t tun_get_user(struct tun_file *tfile,
> >> skb_shinfo(skb)->gso_segs = 0;
> >> }
> >>
> >>- tun = __tun_get(tfile);
> >>- if (!tun)
> >>+ rcu_read_lock();
> >>+ tun = rcu_dereference(tfile->tun);
> >>+ if (!tun) {
> >>+ rcu_read_unlock();
> >> return -EBADFD;
> >>+ }
> >>
> >> switch (tfile->flags& TUN_TYPE_MASK) {
> >> case TUN_TUN_DEV:
> >>@@ -720,26 +808,30 @@ static ssize_t tun_get_user(struct tun_file *tfile,
> >> skb->protocol = eth_type_trans(skb, tun->dev);
> >> break;
> >> }
> >>-
> >>- netif_rx_ni(skb);
> >> tun->dev->stats.rx_packets++;
> >> tun->dev->stats.rx_bytes += len;
> >>- tun_put(tun);
> >>+ rcu_read_unlock();
> >>+
> >>+ netif_rx_ni(skb);
> >>+
> >> return count;
> >>
> >> err_free:
> >> count = -EINVAL;
> >> kfree_skb(skb);
> >> err:
> >>- tun = __tun_get(tfile);
> >>- if (!tun)
> >>+ rcu_read_lock();
> >>+ tun = rcu_dereference(tfile->tun);
> >>+ if (!tun) {
> >>+ rcu_read_unlock();
> >> return -EBADFD;
> >>+ }
> >>
> >> if (drop)
> >> tun->dev->stats.rx_dropped++;
> >> if (error)
> >> tun->dev->stats.rx_frame_errors++;
> >>- tun_put(tun);
> >>+ rcu_read_unlock();
> >> return count;
> >> }
> >>
> >>@@ -833,12 +925,13 @@ static ssize_t tun_put_user(struct tun_file *tfile,
> >> skb_copy_datagram_const_iovec(skb, 0, iv, total, len);
> >> total += skb->len;
> >>
> >>- tun = __tun_get(tfile);
> >>+ rcu_read_lock();
> >>+ tun = rcu_dereference(tfile->tun);
> >> if (tun) {
> >> tun->dev->stats.tx_packets++;
> >> tun->dev->stats.tx_bytes += len;
> >>- tun_put(tun);
> >> }
> >>+ rcu_read_unlock();
> >>
> >> return total;
> >> }
> >>@@ -869,28 +962,31 @@ static ssize_t tun_do_read(struct tun_file *tfile,
> >> break;
> >> }
> >>
> >>- tun = __tun_get(tfile);
> >>+ rcu_read_lock();
> >>+ tun = rcu_dereference(tfile->tun);
> >> if (!tun) {
> >>- ret = -EIO;
> >>+ ret = -EBADFD;
> >BADFD is for when you get passed something like -1 fd.
> >Here fd is OK, it's just in a bad state so you can not do IO.
> >
>
> Sure.
> >>+ rcu_read_unlock();
> >> break;
> >> }
> >> if (tun->dev->reg_state != NETREG_REGISTERED) {
> >> ret = -EIO;
> >>- tun_put(tun);
> >>+ rcu_read_unlock();
> >> break;
> >> }
> >>- tun_put(tun);
> >>+ rcu_read_unlock();
> >>
> >> /* Nothing to read, let's sleep */
> >> schedule();
> >> continue;
> >> }
> >>
> >>- tun = __tun_get(tfile);
> >>+ rcu_read_lock();
> >>+ tun = rcu_dereference(tfile->tun);
> >> if (tun) {
> >> netif_wake_queue(tun->dev);
> >>- tun_put(tun);
> >> }
> >>+ rcu_read_unlock();
> >>
> >> ret = tun_put_user(tfile, skb, iv, len);
> >> kfree_skb(skb);
> >>@@ -1038,6 +1134,9 @@ static int tun_flags(struct tun_struct *tun)
> >> if (tun->flags& TUN_VNET_HDR)
> >> flags |= IFF_VNET_HDR;
> >>
> >>+ if (tun->flags& TUN_TAP_MQ)
> >>+ flags |= IFF_MULTI_QUEUE;
> >>+
> >> return flags;
> >> }
> >>
> >>@@ -1097,8 +1196,7 @@ static int tun_set_iff(struct net *net, struct file *file, struct ifreq *ifr)
> >> err = tun_attach(tun, file);
> >> if (err< 0)
> >> return err;
> >>- }
> >>- else {
> >>+ } else {
> >> char *name;
> >> unsigned long flags = 0;
> >>
> >>@@ -1142,6 +1240,8 @@ static int tun_set_iff(struct net *net, struct file *file, struct ifreq *ifr)
> >> dev->hw_features = NETIF_F_SG | NETIF_F_FRAGLIST |
> >> TUN_USER_FEATURES;
> >> dev->features = dev->hw_features;
> >>+ if (ifr->ifr_flags& IFF_MULTI_QUEUE)
> >>+ dev->features |= NETIF_F_LLTX;
> >>
> >> err = register_netdevice(tun->dev);
> >> if (err< 0)
> >>@@ -1154,7 +1254,7 @@ static int tun_set_iff(struct net *net, struct file *file, struct ifreq *ifr)
> >>
> >> err = tun_attach(tun, file);
> >> if (err< 0)
> >>- goto failed;
> >>+ goto err_free_dev;
> >> }
> >>
> >> tun_debug(KERN_INFO, tun, "tun_set_iff\n");
> >>@@ -1174,6 +1274,11 @@ static int tun_set_iff(struct net *net, struct file *file, struct ifreq *ifr)
> >> else
> >> tun->flags&= ~TUN_VNET_HDR;
> >>
> >>+ if (ifr->ifr_flags& IFF_MULTI_QUEUE)
> >>+ tun->flags |= TUN_TAP_MQ;
> >>+ else
> >>+ tun->flags&= ~TUN_TAP_MQ;
> >>+
> >> /* Cache flags from tun device */
> >> tfile->flags = tun->flags;
> >> /* Make sure persistent devices do not get stuck in
> >>@@ -1187,7 +1292,6 @@ static int tun_set_iff(struct net *net, struct file *file, struct ifreq *ifr)
> >>
> >> err_free_dev:
> >> free_netdev(dev);
> >>-failed:
> >> return err;
> >> }
> >>
> >>@@ -1264,38 +1368,40 @@ static long __tun_chr_ioctl(struct file *file, unsigned int cmd,
> >> (unsigned int __user*)argp);
> >> }
> >>
> >>- rtnl_lock();
> >>-
> >>- tun = __tun_get(tfile);
> >>- if (cmd == TUNSETIFF&& !tun) {
> >>+ ret = 0;
> >>+ if (cmd == TUNSETIFF) {
> >>+ rtnl_lock();
> >> ifr.ifr_name[IFNAMSIZ-1] = '\0';
> >>-
> >> ret = tun_set_iff(tfile->net, file,&ifr);
> >>-
> >>+ rtnl_unlock();
> >> if (ret)
> >>- goto unlock;
> >>-
> >>+ return ret;
> >> if (copy_to_user(argp,&ifr, ifreq_len))
> >>- ret = -EFAULT;
> >>- goto unlock;
> >>+ return -EFAULT;
> >>+ return ret;
> >> }
> >>
> >>+ rtnl_lock();
> >>+
> >>+ rcu_read_lock();
> >>+
> >> ret = -EBADFD;
> >>+ tun = rcu_dereference(tfile->tun);
> >> if (!tun)
> >> goto unlock;
> >>+ else
> >>+ ret = 0;
> >>
> >>- tun_debug(KERN_INFO, tun, "tun_chr_ioctl cmd %d\n", cmd);
> >>-
> >>- ret = 0;
> >> switch (cmd) {
> >> case TUNGETIFF:
> >> ret = tun_get_iff(current->nsproxy->net_ns, tun,&ifr);
> >>+ rcu_read_unlock();
> >> if (ret)
> >>- break;
> >>+ goto out;
> >>
> >> if (copy_to_user(argp,&ifr, ifreq_len))
> >> ret = -EFAULT;
> >>- break;
> >>+ goto out;
> >>
> >> case TUNSETNOCSUM:
> >> /* Disable/Enable checksum */
> >>@@ -1357,9 +1463,10 @@ static long __tun_chr_ioctl(struct file *file, unsigned int cmd,
> >> /* Get hw address */
> >> memcpy(ifr.ifr_hwaddr.sa_data, tun->dev->dev_addr, ETH_ALEN);
> >> ifr.ifr_hwaddr.sa_family = tun->dev->type;
> >>+ rcu_read_unlock();
> >> if (copy_to_user(argp,&ifr, ifreq_len))
> >> ret = -EFAULT;
> >>- break;
> >>+ goto out;
> >>
> >> case SIOCSIFHWADDR:
> >> /* Set hw address */
> >>@@ -1375,9 +1482,9 @@ static long __tun_chr_ioctl(struct file *file, unsigned int cmd,
> >> }
> >>
> >> unlock:
> >>+ rcu_read_unlock();
> >>+out:
> >> rtnl_unlock();
> >>- if (tun)
> >>- tun_put(tun);
> >> return ret;
> >> }
> >>
> >>@@ -1517,6 +1624,11 @@ out:
> >> return ret;
> >> }
> >>
> >>+static void tun_sock_destruct(struct sock *sk)
> >>+{
> >>+ skb_queue_purge(&sk->sk_receive_queue);
> >>+}
> >>+
> >> static int tun_chr_open(struct inode *inode, struct file * file)
> >> {
> >> struct net *net = current->nsproxy->net_ns;
> >>@@ -1540,6 +1652,7 @@ static int tun_chr_open(struct inode *inode, struct file * file)
> >> sock_init_data(&tfile->socket,&tfile->sk);
> >>
> >> tfile->sk.sk_write_space = tun_sock_write_space;
> >>+ tfile->sk.sk_destruct = tun_sock_destruct;
> >> tfile->sk.sk_sndbuf = INT_MAX;
> >> file->private_data = tfile;
> >>
> >>@@ -1549,31 +1662,8 @@ static int tun_chr_open(struct inode *inode, struct file * file)
> >> static int tun_chr_close(struct inode *inode, struct file *file)
> >> {
> >> struct tun_file *tfile = file->private_data;
> >>- struct tun_struct *tun;
> >>-
> >>- tun = __tun_get(tfile);
> >>- if (tun) {
> >>- struct net_device *dev = tun->dev;
> >>-
> >>- tun_debug(KERN_INFO, tun, "tun_chr_close\n");
> >>-
> >>- __tun_detach(tun);
> >>-
> >>- /* If desirable, unregister the netdevice. */
> >>- if (!(tun->flags& TUN_PERSIST)) {
> >>- rtnl_lock();
> >>- if (dev->reg_state == NETREG_REGISTERED)
> >>- unregister_netdevice(dev);
> >>- rtnl_unlock();
> >>- }
> >>
> >>- /* drop the reference that netdevice holds */
> >>- sock_put(&tfile->sk);
> >>-
> >>- }
> >>-
> >>- /* drop the reference that file holds */
> >>- sock_put(&tfile->sk);
> >>+ tun_detach(tfile, true);
> >>
> >> return 0;
> >> }
> >>@@ -1700,14 +1790,17 @@ static void tun_cleanup(void)
> >> * holding a reference to the file for as long as the socket is in use. */
> >> struct socket *tun_get_socket(struct file *file)
> >> {
> >>- struct tun_struct *tun;
> >>+ struct tun_struct *tun = NULL;
> >> struct tun_file *tfile = file->private_data;
> >> if (file->f_op !=&tun_fops)
> >> return ERR_PTR(-EINVAL);
> >>- tun = tun_get(file);
> >>- if (!tun)
> >>+ rcu_read_lock();
> >>+ tun = rcu_dereference(tfile->tun);
> >>+ if (!tun) {
> >>+ rcu_read_unlock();
> >> return ERR_PTR(-EBADFD);
> >>- tun_put(tun);
> >>+ }
> >>+ rcu_read_unlock();
> >> return&tfile->socket;
> >> }
> >> EXPORT_SYMBOL_GPL(tun_get_socket);
^ permalink raw reply
* Re: [PATCH 1/1] use USER_DS in vhost_worker thread
From: Michael S. Tsirkin @ 2012-06-26 11:42 UTC (permalink / raw)
To: Christian Borntraeger; +Cc: kvm, netdev, Jens Freimann
In-Reply-To: <1340708398-17965-2-git-send-email-borntraeger@de.ibm.com>
On Tue, Jun 26, 2012 at 12:59:58PM +0200, Christian Borntraeger wrote:
> From: Jens Freimann <jfrei@linux.vnet.ibm.com>
>
> On some architectures address spaces are set up in a way that this is
> not necessary to work properly but on some others (like s390) it is.
> Make sure we operate on the user address space to allow copy_xxx_user()
> from the vhost_worker() thread by setting it explicitly before calling
> use_mm() and revert it after unuse_mm().
>
> Signed-off-by: Jens Freimann <jfrei@linux.vnet.ibm.com>
> Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
Acked-by: Michael S. Tsirkin <mst@redhat.com>
Dave, can you queue this up for 3.5 please?
Thanks.
> ---
> drivers/vhost/vhost.c | 3 +++
> 1 files changed, 3 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
> index 94dbd25..112156f 100644
> --- a/drivers/vhost/vhost.c
> +++ b/drivers/vhost/vhost.c
> @@ -191,7 +191,9 @@ static int vhost_worker(void *data)
> struct vhost_dev *dev = data;
> struct vhost_work *work = NULL;
> unsigned uninitialized_var(seq);
> + mm_segment_t oldfs = get_fs();
>
> + set_fs(USER_DS);
> use_mm(dev->mm);
>
> for (;;) {
> @@ -229,6 +231,7 @@ static int vhost_worker(void *data)
>
> }
> unuse_mm(dev->mm);
> + set_fs(oldfs);
> return 0;
> }
>
> --
> 1.7.0.4
^ permalink raw reply
* Re: [PATCH 3/3] net: fec: add phy-reset-interval for device tree probe
From: Shawn Guo @ 2012-06-26 11:15 UTC (permalink / raw)
To: Lothar Waßmann; +Cc: David S. Miller, netdev, linux-arm-kernel
In-Reply-To: <20457.31943.332545.171232@ipc1.ka-ro>
On Tue, Jun 26, 2012 at 11:11:35AM +0200, Lothar Waßmann wrote:
> Hi,
>
> Shawn Guo writes:
> > Different boards may require different phy reset interval time. Add
> > property phy-reset-interval for device tree probe, so that the boards
> > that need a longer interval time can specify it in their device tree.
> >
> 'phy-reset-duration' would be a more appropriate name.
>
Ok, point taken. Thanks.
--
Regards,
Shawn
^ 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