* Re: [PATCH] [net]ip_tunnel: the lack of vti_link_ops' dellink() cause kernel panic
From: Nicolas Dichtel @ 2014-11-18 17:23 UTC (permalink / raw)
To: Xin Long, network dev; +Cc: Steffen Klassert
In-Reply-To: <1416307059-32732-1-git-send-email-lucien.xin@gmail.com>
Le 18/11/2014 11:37, Xin Long a écrit :
> Now the vti_link_ops do not point the .dellink, rtnl_newlink will invoke
> the default function, unregister_netdevice_queue, which will cause the
> dev to unregister later. so when we delete a vti device, the net_device
> will be removed, but the tunnel still in the tunnel list. then, we add
> a new vti, in ip_tunnel_find():
>
> hlist_for_each_entry_rcu(t, head, hash_node) {
> if (local == t->parms.iph.saddr &&
> remote == t->parms.iph.daddr &&
> link == t->parms.link &&
> type == t->dev->type &&
> ip_tunnel_key_match(&t->parms, flags, key))
> break;
> }
>
> the dev of ip_tunnel *t may be null because of unregister_netdevice_queue
> motioned above. so the panic will happen:
>
> [ 3835.072977] IP: [<ffffffffa04103fd>] ip_tunnel_find+0x9d/0xc0 [ip_tunnel]
> [ 3835.073008] PGD b2c21067 PUD b7277067 PMD 0
> [ 3835.073008] Oops: 0000 [#1] SMP
> .....
> [ 3835.073008] Stack:
> [ 3835.073008] ffff8800b72d77f0 ffffffffa0411924 ffff8800bb956000 ffff8800b72d78e0
> [ 3835.073008] ffff8800b72d78a0 0000000000000000 ffffffffa040d100 ffff8800b72d7858
> [ 3835.073008] ffffffffa040b2e3 0000000000000000 0000000000000000 0000000000000000
> [ 3835.073008] Call Trace:
> [ 3835.073008] [<ffffffffa0411924>] ip_tunnel_newlink+0x64/0x160 [ip_tunnel]
> [ 3835.073008] [<ffffffffa040b2e3>] vti_newlink+0x43/0x70 [ip_vti]
> [ 3835.073008] [<ffffffff8150d4da>] rtnl_newlink+0x4fa/0x5f0
> [ 3835.073008] [<ffffffff812f68bb>] ? nla_strlcpy+0x5b/0x70
> [ 3835.073008] [<ffffffff81508fb0>] ? rtnl_link_ops_get+0x40/0x60
> [ 3835.073008] [<ffffffff8150d11f>] ? rtnl_newlink+0x13f/0x5f0
> [ 3835.073008] [<ffffffff81509cf4>] rtnetlink_rcv_msg+0xa4/0x270
> [ 3835.073008] [<ffffffff8126adf5>] ? sock_has_perm+0x75/0x90
> [ 3835.073008] [<ffffffff81509c50>] ? rtnetlink_rcv+0x30/0x30
> [ 3835.073008] [<ffffffff81529e39>] netlink_rcv_skb+0xa9/0xc0
> [ 3835.073008] [<ffffffff81509c48>] rtnetlink_rcv+0x28/0x30
> ....
>
> the reproduction can be like this:
>
> modprobe ip_vti
> ip link del ip_vti0 type vti
> ip link add ip_vti0 type vti
> rmmod ip_vti
>
> do that one or more time, kernel will panic.
>
> Signed-off-by: Xin Long <lucien.xin@gmail.com>
A quick look at the ipv6 side seems to show that there is the same problem. Can
you provide the IPv6 patch too?
Note also that the maintainer of this module is Steffen Klassert, please don't
forget to CC him.
> ---
> net/ipv4/ip_vti.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/net/ipv4/ip_vti.c b/net/ipv4/ip_vti.c
> index 3e86101..1a7e979 100644
> --- a/net/ipv4/ip_vti.c
> +++ b/net/ipv4/ip_vti.c
> @@ -528,6 +528,7 @@ static struct rtnl_link_ops vti_link_ops __read_mostly = {
> .validate = vti_tunnel_validate,
> .newlink = vti_newlink,
> .changelink = vti_changelink,
> + .dellink = ip_tunnel_dellink,
Nitpicking: other lines into this struct uses tabs to align the '=', but
the one you add uses spaces.
Thank you,
Nicolas
^ permalink raw reply
* Re: [PATCH net-next] tun: return NET_XMIT_DROP for dropped packets
From: Amos Kong @ 2014-11-18 16:53 UTC (permalink / raw)
To: Jason Wang; +Cc: davem, netdev, linux-kernel, Michael S. Tsirkin
In-Reply-To: <1416288041-30921-1-git-send-email-jasowang@redhat.com>
[-- Attachment #1: Type: text/plain, Size: 1416 bytes --]
On Tue, Nov 18, 2014 at 01:20:41PM +0800, Jason Wang wrote:
> After commit 5d097109257c03a71845729f8db6b5770c4bbedc
> ("tun: only queue packets on device"), NETDEV_TX_OK was returned for
> dropped packets. This will confuse pktgen since dropped packets were
> counted as sent ones.
>
> Fixing this by returning NET_XMIT_DROP to let pktgen count it as error
> packet.
>
> Cc: Michael S. Tsirkin <mst@redhat.com>
> Signed-off-by: Jason Wang <jasowang@redhat.com>
> ---
> drivers/net/tun.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/net/tun.c b/drivers/net/tun.c
> index e3fa65a..ac53a73 100644
> --- a/drivers/net/tun.c
> +++ b/drivers/net/tun.c
> @@ -819,7 +819,7 @@ drop:
> skb_tx_error(skb);
> kfree_skb(skb);
> rcu_read_unlock();
> - return NETDEV_TX_OK;
> + return NET_XMIT_DROP;
Quoted from linux/drivers/firewire/net.c:
/*
* FIXME: According to a patch from 2003-02-26, "returning non-zero
* causes serious problems" here, allegedly. Before that patch,
* -ERRNO was returned which is not appropriate under Linux 2.6.
* Perhaps more needs to be done? Stop the queue in serious
* conditions and restart it elsewhere?
*/
I saw many drivers return NETDEV_TX_OK in xmit for drop packets, eg: virtio_net.c
> }
>
> static void tun_net_mclist(struct net_device *dev)
> --
> 1.9.1
--
Amos.
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH 2/4] arch: Add lightweight memory barriers fast_rmb() and fast_wmb()
From: Will Deacon @ 2014-11-18 16:48 UTC (permalink / raw)
To: Alexander Duyck
Cc: Benjamin Herrenschmidt, Alexander Duyck,
linux-arch@vger.kernel.org, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org, mathieu.desnoyers@polymtl.ca,
peterz@infradead.org, heiko.carstens@de.ibm.com, mingo@kernel.org,
mikey@neuling.org, linux@arm.linux.org.uk,
donald.c.skidmore@intel.com, matthew.vick@intel.com,
geert@linux-m68k.org, jeffrey.t.kirsher@intel.com,
romieu@fr.zoreil.com, paulmck@linux.vnet.ibm.com
In-Reply-To: <546B71DE.4050506@redhat.com>
On Tue, Nov 18, 2014 at 04:20:46PM +0000, Alexander Duyck wrote:
> On 11/18/2014 03:58 AM, Will Deacon wrote:
> > So actually, this is an interesting case where the barrier would like to
> > know whether the memory returned by dma_alloc_coherent is h/w coherent
> > (normal, cacheable) or s/w coherent (normal, non-cacheable). I think Ben
> > is thinking of the h/w coherent case (i.e. actual snooping into the CPU
> > caches by the DMA master).
> >
> > For the former, we could use inner-shareable barriers. For the latter, we'd
> > need to use outer-shareable barriers.
> >
> > If we can't tell, then these should be dmb(osh), which will work for both.
> >
>
> Okay, so I will update the ARM portion of my patches to use osh and
> oshst then since it sounds like I was using too strong of barriers.
Sounds good. Another reason this is interesting is because the native
acquire/release instructions on ARMv8 actually take into account the
shareability domain of the virtual address, so using them would give you
the shareability domain you want but slightly stronger ordering guarantees
within that domain.
Still, either of them will be a damn sight better than the dsb we currently
have courtesy of the mandatory barriers.
Will
^ permalink raw reply
* Re: [PATCH 2/2] net: can: comparison of unsigned variable
From: Michal Simek @ 2014-11-18 16:47 UTC (permalink / raw)
To: Sudip Mukherjee, Wolfgang Grandegger, Marc Kleine-Budde,
Michal Simek, Sören Brinkmann
Cc: netdev, linux-kernel, linux-arm-kernel, linux-can
In-Reply-To: <1416318427-28676-2-git-send-email-sudipm.mukherjee@gmail.com>
On 11/18/2014 02:47 PM, Sudip Mukherjee wrote:
> err was of the type u32. it was being compared with < 0, and being
> an unsigned variable the comparison would have been always false.
>
> moreover, err was getting the return value from set_reset_mode()
> and xcan_set_bittiming(), and both are returning int.
>
> Signed-off-by: Sudip Mukherjee <sudip@vectorindia.org>
> ---
> drivers/net/can/xilinx_can.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/can/xilinx_can.c b/drivers/net/can/xilinx_can.c
> index 72fe96f..67c2dd4 100644
> --- a/drivers/net/can/xilinx_can.c
> +++ b/drivers/net/can/xilinx_can.c
> @@ -300,7 +300,8 @@ static int xcan_set_bittiming(struct net_device *ndev)
> static int xcan_chip_start(struct net_device *ndev)
> {
> struct xcan_priv *priv = netdev_priv(ndev);
> - u32 err, reg_msr, reg_sr_mask;
> + u32 reg_msr, reg_sr_mask;
> + int err;
> unsigned long timeout;
>
> /* Check if it is in reset mode */
>
Reviewed-by: Michal Simek <michal.simek@xilinx.com>
Thanks,
Michal
^ permalink raw reply
* Re: [PATCH 2/4] arch: Add lightweight memory barriers fast_rmb() and fast_wmb()
From: Alexander Duyck @ 2014-11-18 16:20 UTC (permalink / raw)
To: Will Deacon
Cc: Benjamin Herrenschmidt, Alexander Duyck,
linux-arch@vger.kernel.org, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org, mathieu.desnoyers@polymtl.ca,
peterz@infradead.org, heiko.carstens@de.ibm.com, mingo@kernel.org,
mikey@neuling.org, linux@arm.linux.org.uk,
donald.c.skidmore@intel.com, matthew.vick@intel.com,
geert@linux-m68k.org, jeffrey.t.kirsher@intel.com,
romieu@fr.zoreil.com, paulmck@linux.vnet.ibm.com
In-Reply-To: <20141118115836.GL18842@arm.com>
On 11/18/2014 03:58 AM, Will Deacon wrote:
> On Tue, Nov 18, 2014 at 03:13:29AM +0000, Alexander Duyck wrote:
>> On 11/17/2014 04:39 PM, Benjamin Herrenschmidt wrote:
>>> On Mon, 2014-11-17 at 12:24 -0800, Alexander Duyck wrote:
>>>> Yes and no. So for example on ARM I used the dmb() operation, however
>>>> I
>>>> have to use the barrier at the system level instead of just the inner
>>>> shared domain. However on many other architectures they are just the
>>>> same as the smp_* variants.
>>>>
>>>> Basically the resultant code is somewhere between the smp and non-smp
>>>> barriers in terms of what they cover.
>>> There I don't quite follow you. You need to explain better especially in
>>> the documentation because otherwise people will get it wrong...
>>>
>>> If it's ordering in the coherent domain, I fail to see how a DMA agent
>>> is different than another processor when it comes to barriers, so I fail
>>> to see the difference with smp_*
>>>
>>> I understand the MMIO vs. memory issue, we do have the same on powerpc,
>>> but that other aspect eludes me.
>>>
>> ARM adds some funky things. They have two different types of
>> primitives, a dmb() which is a data memory barrier, and a dsb() which is
>> a data synchronization barrier. Then with each of those they have the
>> "domains" the barriers are effective within.
>>
>> So for example on ARM a rmb() is dsb(sy) which means it is a system wide
>> synchronization barrier which stops execution on the CPU core until the
>> read completes. However the smp_rmb() is a dmb(ish) which means it is
>> only a barrier as far as the inner shareable domain which I believe only
>> goes as far as the local shared cache hierarchy and only guarantees read
>> ordering without necessarily halting the CPU or stopping in-order
>> speculative reads. So what a coherent_rmb() would be in my setup is
>> dmb(sy) which means the barrier runs all the way out to memory, and it
>> is allowed to speculative read as long as it does it in order.
>>
>> If it is still unclear you might check out Will Deacon's talk on the
>> topic at https://www.youtube.com/watch?v=6ORn6_35kKo, at about 7:00 in
>> he explains the whole domains thing, and at 13:30 he explains dmb()/dsb().
> So actually, this is an interesting case where the barrier would like to
> know whether the memory returned by dma_alloc_coherent is h/w coherent
> (normal, cacheable) or s/w coherent (normal, non-cacheable). I think Ben
> is thinking of the h/w coherent case (i.e. actual snooping into the CPU
> caches by the DMA master).
>
> For the former, we could use inner-shareable barriers. For the latter, we'd
> need to use outer-shareable barriers.
>
> If we can't tell, then these should be dmb(osh), which will work for both.
>
> Will
Okay, so I will update the ARM portion of my patches to use osh and
oshst then since it sounds like I was using too strong of barriers.
- Alex
^ permalink raw reply
* [PATCH v2] usbnet: rtl8150: remove unused variable
From: Sudip Mukherjee @ 2014-11-18 16:25 UTC (permalink / raw)
To: Petko Manolov; +Cc: Sudip Mukherjee, linux-usb, netdev, linux-kernel
remove unused variable
Signed-off-by: Sudip Mukherjee <sudip@vectorindia.org>
---
change in v2: changed the commit message
drivers/net/usb/rtl8150.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/net/usb/rtl8150.c b/drivers/net/usb/rtl8150.c
index 6e87e57..d37b7dc 100644
--- a/drivers/net/usb/rtl8150.c
+++ b/drivers/net/usb/rtl8150.c
@@ -753,14 +753,13 @@ static int rtl8150_open(struct net_device *netdev)
static int rtl8150_close(struct net_device *netdev)
{
rtl8150_t *dev = netdev_priv(netdev);
- int res = 0;
netif_stop_queue(netdev);
if (!test_bit(RTL8150_UNPLUG, &dev->flags))
disable_net_traffic(dev);
unlink_all_urbs(dev);
- return res;
+ return 0;
}
static void rtl8150_get_drvinfo(struct net_device *netdev, struct ethtool_drvinfo *info)
--
1.8.1.2
^ permalink raw reply related
* [PATCH net-next 4/4] tuntap: Increase the number of queues in tun.
From: Pankaj Gupta @ 2014-11-18 16:22 UTC (permalink / raw)
To: linux-kernel, netdev
Cc: davem, jasowang, mst, dgibson, vfalico, edumazet, vyasevic, hkchu,
wuzhy, xemul, therbert, bhutchings, xii, stephen, jiri,
sergei.shtylyov, Pankaj Gupta
In-Reply-To: <1416327778-17716-1-git-send-email-pagupta@redhat.com>
Networking under kvm works best if we allocate a per-vCPU RX and TX
queue in a virtual NIC. This requires a per-vCPU queue on the host side.
It is now safe to increase the maximum number of queues.
Preceding patches:
net: allow large number of rx queues
tuntap: Reduce the size of tun_struct by using flex array
tuntap: Accepts tuntap max queue length as net sysctl parameter
made sure this won't cause failures due to high order memory
allocations. Increase it to 256: this is the max number of vCPUs
KVM supports.
Signed-off-by: Pankaj Gupta <pagupta@redhat.com>
Reviewed-by: David Gibson <dgibson@redhat.com>
---
drivers/net/tun.c | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/drivers/net/tun.c b/drivers/net/tun.c
index a9b3eb4..2fb31b7 100644
--- a/drivers/net/tun.c
+++ b/drivers/net/tun.c
@@ -115,10 +115,11 @@ struct tap_filter {
unsigned char addr[FLT_EXACT_COUNT][ETH_ALEN];
};
-/* DEFAULT_MAX_NUM_RSS_QUEUES were chosen to let the rx/tx queues allocated for
- * the netdevice to be fit in one page. So we can make sure the success of
- * memory allocation. TODO: increase the limit. */
-#define MAX_TAP_QUEUES DEFAULT_MAX_NUM_RSS_QUEUES
+/* MAX_TAP_QUEUES 256 is chosen to allow rx/tx queues to be equal
+ * to max number of vCPUS in guest. Also, we are making sure here
+ * queue memory allocation do not fail.
+ */
+#define MAX_TAP_QUEUES 256
#define MIN_TAP_QUEUES 1
#define MAX_TAP_FLOWS 4096
--
1.8.3.1
^ permalink raw reply related
* [PATCH net-next 3/4] tuntap: reduce the size of tun_struct by using flex array.
From: Pankaj Gupta @ 2014-11-18 16:22 UTC (permalink / raw)
To: linux-kernel, netdev
Cc: davem, jasowang, mst, dgibson, vfalico, edumazet, vyasevic, hkchu,
wuzhy, xemul, therbert, bhutchings, xii, stephen, jiri,
sergei.shtylyov, Pankaj Gupta
In-Reply-To: <1416327778-17716-1-git-send-email-pagupta@redhat.com>
This patch switches to flex array to implement the flow caches, it brings
several advantages:
- Reduce the size of the tun_struct structure, which allows us to increase the
upper limit of queues in future.
- Avoid higher order memory allocation. It will be useful when switching to
pure hashing in flow cache which may demand a larger size array in future.
After this patch, the size of tun_struct on x86_64 reduced from 8512 to
328
Signed-off-by: Jason Wang <jasowang@redhat.com>
Signed-off-by: Pankaj Gupta <pagupta@redhat.com>
Reviewed-by: David Gibson <dgibson@redhat.com>
---
drivers/net/tun.c | 49 +++++++++++++++++++++++++++++++++++++------------
1 file changed, 37 insertions(+), 12 deletions(-)
diff --git a/drivers/net/tun.c b/drivers/net/tun.c
index e3fa65a..bd07a6d 100644
--- a/drivers/net/tun.c
+++ b/drivers/net/tun.c
@@ -65,6 +65,7 @@
#include <linux/nsproxy.h>
#include <linux/virtio_net.h>
#include <linux/rcupdate.h>
+#include <linux/flex_array.h>
#include <net/ipv6.h>
#include <net/net_namespace.h>
#include <net/netns/generic.h>
@@ -188,7 +189,7 @@ struct tun_struct {
int debug;
#endif
spinlock_t lock;
- struct hlist_head flows[TUN_NUM_FLOW_ENTRIES];
+ struct flex_array *flows;
struct timer_list flow_gc_timer;
unsigned long ageing_time;
unsigned int numdisabled;
@@ -249,10 +250,11 @@ static void tun_flow_flush(struct tun_struct *tun)
spin_lock_bh(&tun->lock);
for (i = 0; i < TUN_NUM_FLOW_ENTRIES; i++) {
+ struct hlist_head *h = flex_array_get(tun->flows, i);
struct tun_flow_entry *e;
struct hlist_node *n;
- hlist_for_each_entry_safe(e, n, &tun->flows[i], hash_link)
+ hlist_for_each_entry_safe(e, n, h, hash_link)
tun_flow_delete(tun, e);
}
spin_unlock_bh(&tun->lock);
@@ -264,10 +266,11 @@ static void tun_flow_delete_by_queue(struct tun_struct *tun, u16 queue_index)
spin_lock_bh(&tun->lock);
for (i = 0; i < TUN_NUM_FLOW_ENTRIES; i++) {
+ struct hlist_head *h = flex_array_get(tun->flows, i);
struct tun_flow_entry *e;
struct hlist_node *n;
- hlist_for_each_entry_safe(e, n, &tun->flows[i], hash_link) {
+ hlist_for_each_entry_safe(e, n, h, hash_link) {
if (e->queue_index == queue_index)
tun_flow_delete(tun, e);
}
@@ -287,10 +290,11 @@ static void tun_flow_cleanup(unsigned long data)
spin_lock_bh(&tun->lock);
for (i = 0; i < TUN_NUM_FLOW_ENTRIES; i++) {
+ struct hlist_head *h = flex_array_get(tun->flows, i);
struct tun_flow_entry *e;
struct hlist_node *n;
- hlist_for_each_entry_safe(e, n, &tun->flows[i], hash_link) {
+ hlist_for_each_entry_safe(e, n, h, hash_link) {
unsigned long this_timer;
count++;
this_timer = e->updated + delay;
@@ -317,7 +321,7 @@ static void tun_flow_update(struct tun_struct *tun, u32 rxhash,
if (!rxhash)
return;
else
- head = &tun->flows[tun_hashfn(rxhash)];
+ head = flex_array_get(tun->flows, tun_hashfn(rxhash));
rcu_read_lock();
@@ -380,7 +384,8 @@ static u16 tun_select_queue(struct net_device *dev, struct sk_buff *skb,
txq = skb_get_hash(skb);
if (txq) {
- e = tun_flow_find(&tun->flows[tun_hashfn(txq)], txq);
+ e = tun_flow_find(flex_array_get(tun->flows,
+ tun_hashfn(txq)), txq);
if (e) {
tun_flow_save_rps_rxhash(e, txq);
txq = e->queue_index;
@@ -760,8 +765,8 @@ static netdev_tx_t tun_net_xmit(struct sk_buff *skb, struct net_device *dev)
rxhash = skb_get_hash(skb);
if (rxhash) {
struct tun_flow_entry *e;
- e = tun_flow_find(&tun->flows[tun_hashfn(rxhash)],
- rxhash);
+ e = tun_flow_find(flex_array_get(tun->flows,
+ tun_hashfn(rxhash)), rxhash);
if (e)
tun_flow_save_rps_rxhash(e, rxhash);
}
@@ -896,23 +901,40 @@ static const struct net_device_ops tap_netdev_ops = {
#endif
};
-static void tun_flow_init(struct tun_struct *tun)
+static int tun_flow_init(struct tun_struct *tun)
{
- int i;
+ struct flex_array *buckets;
+ int i, err;
+
+ buckets = flex_array_alloc(sizeof(struct hlist_head),
+ TUN_NUM_FLOW_ENTRIES, GFP_KERNEL);
+ if (!buckets)
+ return -ENOMEM;
+
+ err = flex_array_prealloc(buckets, 0, TUN_NUM_FLOW_ENTRIES, GFP_KERNEL);
+ if (err) {
+ flex_array_free(buckets);
+ return -ENOMEM;
+ }
+ tun->flows = buckets;
for (i = 0; i < TUN_NUM_FLOW_ENTRIES; i++)
- INIT_HLIST_HEAD(&tun->flows[i]);
+ INIT_HLIST_HEAD((struct hlist_head *)
+ flex_array_get(buckets, i));
tun->ageing_time = TUN_FLOW_EXPIRE;
setup_timer(&tun->flow_gc_timer, tun_flow_cleanup, (unsigned long)tun);
mod_timer(&tun->flow_gc_timer,
round_jiffies_up(jiffies + tun->ageing_time));
+
+ return 0;
}
static void tun_flow_uninit(struct tun_struct *tun)
{
del_timer_sync(&tun->flow_gc_timer);
tun_flow_flush(tun);
+ flex_array_free(tun->flows);
}
/* Initialize net device. */
@@ -1674,7 +1696,10 @@ static int tun_set_iff(struct net *net, struct file *file, struct ifreq *ifr)
goto err_free_dev;
tun_net_init(dev);
- tun_flow_init(tun);
+
+ err = tun_flow_init(tun);
+ if (err < 0)
+ goto err_free_dev;
dev->hw_features = NETIF_F_SG | NETIF_F_FRAGLIST |
TUN_USER_FEATURES | NETIF_F_HW_VLAN_CTAG_TX |
--
1.8.3.1
^ permalink raw reply related
* [PATCH net-next 2/4] tuntap: Accept tuntap maximum number of queues as sysctl
From: Pankaj Gupta @ 2014-11-18 16:22 UTC (permalink / raw)
To: linux-kernel, netdev
Cc: davem, jasowang, mst, dgibson, vfalico, edumazet, vyasevic, hkchu,
wuzhy, xemul, therbert, bhutchings, xii, stephen, jiri,
sergei.shtylyov, Pankaj Gupta
In-Reply-To: <1416327778-17716-1-git-send-email-pagupta@redhat.com>
This patch accepts maximum number of tun/tap queues allocated as
sysctl entry which a user space application like libvirt
can make use of to limit maximum number of tuntap queues.
Value of sysctl entry is writable dynamically.
If no value is set for sysctl entry 'net.tuntap.max_queues'
a default value 256 is used which is equal to maximum number
of vCPUS allowed by KVM.
Signed-off-by: Pankaj Gupta <pagupta@redhat.com>
---
drivers/net/tun.c | 33 +++++++++++++++++++++++++++++++--
1 file changed, 31 insertions(+), 2 deletions(-)
diff --git a/drivers/net/tun.c b/drivers/net/tun.c
index e3fa65a..b03a745 100644
--- a/drivers/net/tun.c
+++ b/drivers/net/tun.c
@@ -71,6 +71,7 @@
#include <net/rtnetlink.h>
#include <net/sock.h>
#include <linux/seq_file.h>
+#include <linux/sysctl.h>
#include <linux/uio.h>
#include <asm/uaccess.h>
@@ -117,10 +118,16 @@ struct tap_filter {
* the netdevice to be fit in one page. So we can make sure the success of
* memory allocation. TODO: increase the limit. */
#define MAX_TAP_QUEUES DEFAULT_MAX_NUM_RSS_QUEUES
+#define MIN_TAP_QUEUES 1
#define MAX_TAP_FLOWS 4096
#define TUN_FLOW_EXPIRE (3 * HZ)
+static struct ctl_table_header *tun_sysctl_header;
+static int tun_queues = MAX_TAP_QUEUES;
+static int min_queues = MIN_TAP_QUEUES;
+static int max_queues = MAX_TAP_QUEUES;
+
/* A tun_file connects an open character device to a tuntap netdevice. It
* also contains all socket related structures (except sock_fprog and tap_filter)
* to serve as one transmit queue for tuntap device. The sock_fprog and
@@ -197,6 +204,19 @@ struct tun_struct {
u32 flow_count;
};
+static struct ctl_table tun_ctl_table[] = {
+ {
+ .procname = "tun_max_queues",
+ .data = &tun_queues,
+ .maxlen = sizeof(int),
+ .mode = 0644,
+ .proc_handler = proc_dointvec_minmax,
+ .extra1 = &min_queues,
+ .extra2 = &max_queues
+ },
+ { }
+};
+
static inline u32 tun_hashfn(u32 rxhash)
{
return rxhash & 0x3ff;
@@ -547,7 +567,7 @@ static int tun_attach(struct tun_struct *tun, struct file *file, bool skip_filte
err = -E2BIG;
if (!tfile->detached &&
- tun->numqueues + tun->numdisabled == MAX_TAP_QUEUES)
+ tun->numqueues + tun->numdisabled == tun_queues)
goto out;
err = 0;
@@ -1624,7 +1644,7 @@ static int tun_set_iff(struct net *net, struct file *file, struct ifreq *ifr)
char *name;
unsigned long flags = 0;
int queues = ifr->ifr_flags & IFF_MULTI_QUEUE ?
- MAX_TAP_QUEUES : 1;
+ tun_queues : 1;
if (!ns_capable(net->user_ns, CAP_NET_ADMIN))
return -EPERM;
@@ -2335,6 +2355,13 @@ static int __init tun_init(void)
pr_info("%s, %s\n", DRV_DESCRIPTION, DRV_VERSION);
pr_info("%s\n", DRV_COPYRIGHT);
+ tun_sysctl_header = register_net_sysctl(&init_net, "net/tuntap",
+ tun_ctl_table);
+
+ if (!tun_sysctl_header)
+ pr_err("Can't register tun_ctl_table. Tun device queue"
+ "setting to default value : %d queues.\n", tun_queues);
+
ret = rtnl_link_register(&tun_link_ops);
if (ret) {
pr_err("Can't register link_ops\n");
@@ -2357,6 +2384,8 @@ static void tun_cleanup(void)
{
misc_deregister(&tun_miscdev);
rtnl_link_unregister(&tun_link_ops);
+ if (tun_sysctl_header)
+ unregister_net_sysctl_table(tun_sysctl_header);
}
/* Get an underlying socket object from tun file. Returns error unless file is
--
1.8.3.1
^ permalink raw reply related
* [PATCH net-net 0/4] Increase the limit of tuntap queues
From: Pankaj Gupta @ 2014-11-18 16:22 UTC (permalink / raw)
To: linux-kernel, netdev
Cc: davem, jasowang, mst, dgibson, vfalico, edumazet, vyasevic, hkchu,
wuzhy, xemul, therbert, bhutchings, xii, stephen, jiri,
sergei.shtylyov, Pankaj Gupta
This patch series is followup to the RFC posted as:
https://lkml.org/lkml/2014/8/18/392
Changes from RFC are:
PATCH 1: Sergei Shtylyov - Add an empty line after declarations.
PATCH 2: Jiri Pirko - Do not introduce new module paramaters.
Michael.S.Tsirkin - We can use sysctl for limiting max number
of queues.
Networking under KVM works best if we allocate a per-vCPU rx and tx
queue in a virtual NIC. This requires a per-vCPU queue on the host side.
Modern physical NICs have multiqueue support for large number of queues.
To scale vNIC to run multiple queues parallel to maximum number of vCPU's
we need to increase number of queues support in tuntap.
This series is to increase the limit of tuntap queues. Original work is being
done by 'jasowang@redhat.com'. I am taking this 'https://lkml.org/lkml/2013/6/19/29'
patch series as a reference. As per discussion in the patch series:
There were two reasons which prevented us from increasing number of tun queues:
- The netdev_queue array in netdevice were allocated through kmalloc, which may
cause a high order memory allocation too when we have several queues.
E.g. sizeof(netdev_queue) is 320, which means a high order allocation would
happens when the device has more than 16 queues.
- We store the hash buckets in tun_struct which results a very large size of
tun_struct, this high order memory allocation fail easily when the memory is
fragmented.
The patch 60877a32bce00041528576e6b8df5abe9251fa73 increases the number of tx
queues. Memory allocation fallback to vzalloc() when kmalloc() fails.
This series tries to address following issues:
- Increase the number of netdev_queue queues for rx similarly its done for tx
queues by falling back to vzalloc() when memory allocation with kmalloc() fails.
- Switches to use flex array to implement the flow caches to avoid higher order
allocations.
- Accept maximum number of queues as sysctl param so that any user space
application like libvirt can use this value to limit number of queues. Also
Administrators can specify maximum number of queues by updating this sysctl
entry.
- Increase number of queues to 256, maximum number is equal to maximum number
of vCPUS allowed in a guest.
I have done some testing to find out any regression and with sample program
which creates tun/tap for single queue / multiqueue device and it seems to be
working fine. I will also post the performance numbers.
tuntap: Increase the number of queues in tun
tuntap: Reduce the size of tun_struct by using flex array
tuntap: Accept tuntap max queue length as sysctl entry
net: allow large number of rx queues
drivers/net/tun.c | 91 +++++++++++++++++++++++++++++++++++---------
net/core/dev.c | 19 ++++++---
2 files changed, 86 insertions(+), 24 deletions(-)
^ permalink raw reply
* Re: [PATCH 1/1 net-next] wireless: remove unnecessary sizeof(u8)
From: Larry Finger @ 2014-11-18 16:12 UTC (permalink / raw)
To: Gheorhios, John W. Linville
Cc: Fabian Frederick, Emmanuel Grumbach, Stefano Brivio,
Johannes Berg, Julian Calaby, linux-wireless,
linux-kernel@vger.kernel.org, Intel Linux Wireless, Chaoming Li,
b43-dev, netdev
In-Reply-To: <CAFpvzpxYrjR_tSF99iosMGZTAmt=_6o_aA+iYjNhxRGJ9aJ6oA@mail.gmail.com>
On 11/18/2014 07:34 AM, Gheorhios wrote:
> Anyone could gently send the link for downloading B43 linux drivers for
> this procedure?
>
>
>
> Then transfer it over to your Ubuntu box.
>
> Now in your Ubuntu Box [computer] please make your way to your Home folder.
>
> Once you are at your home folder right click on your home folder and make a
> new folder and call it wireless.
>
> Now that you have made a new folder called wireless in your home directory,
> it is time to move the downloaded file into the new folder called wireless.
>
> Move The Wireless Folder To The Firmware Directory
>
> sudo cp -r ~/wireless/* /lib/firmware/
>
> Now let's double check to make sure the download made it to the firmware
> directory. To do this type this into the terminal:
>
> ls /lib/firmware
>
> Ok so now that the download is in the firmware directory we need to go to
> that directory. To go there open your terminal and type in:
>
> cd /lib/firmware
>
> Now that you have changed directories let's double check to make sure you
> are in the right directory, this next code tells us where we are in the
> computer file directory. This next code stands for "print working
> directory".
>
> pwd
>
> Are you at /lib/firmware if so good if not go back one step.
>
> Now that we are in the firmware directory. We have to extract the download,
> to do this type in:
>
> sudo -s
>
> Then enter your password then:
>
> tar xvf b43-all-fw.tar_.gz
>
> Now is the firmware extracted properly? check by typing:
>
> ls /lib/firmware/b43
>
> or:
>
> ls /lib/firmware/b43legacy
>
> Do you see the ucode files? if so then delete the gz file:
>
> sudo rm *.gz
>
> Then:
>
> exit
>
> Reboot
No, I do not know where that file is found. Even if I knew of such a file,
Broadcom has expressly declined to provide that firmware for redistribution.
Posting such a file could invite legal action. If someone else has violated
Broadcom's directive, I would not facilitate that violation.
Rather than doing that, the link at
http://wireless.kernel.org/en/users/Drivers/b43#firmwareinstallation shows what
to do for Ubuntu installations. Follow those instructions - they refer to a
legal way to get the firmware. I would have thought that asking this question on
an Ubuntu Forum would have been more productive.
By the way, piggybacking your request on this thread is very bad netiquette. You
should not have done a "reply-to".
Larry
^ permalink raw reply
* Re: [PATCH] usbnet: rtl8150: remove unused variable
From: Petko Manolov @ 2014-11-18 15:30 UTC (permalink / raw)
To: Sudip Mukherjee; +Cc: linux-usb, netdev, linux-kernel
In-Reply-To: <1416315599-16939-1-git-send-email-sudipm.mukherjee@gmail.com>
On 14-11-18 18:29:59, Sudip Mukherjee wrote:
> we were just returning the initial value of res, instead now
> we are returning the value directly.
Looks OK, but could you please fix the wording of the commit message to something like:
Remove unused variable.
This is a tiny patch and the code speaks for itself.
Petko
> Signed-off-by: Sudip Mukherjee <sudip@vectorindia.org>
> ---
> drivers/net/usb/rtl8150.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/drivers/net/usb/rtl8150.c b/drivers/net/usb/rtl8150.c
> index 6e87e57..d37b7dc 100644
> --- a/drivers/net/usb/rtl8150.c
> +++ b/drivers/net/usb/rtl8150.c
> @@ -753,14 +753,13 @@ static int rtl8150_open(struct net_device *netdev)
> static int rtl8150_close(struct net_device *netdev)
> {
> rtl8150_t *dev = netdev_priv(netdev);
> - int res = 0;
>
> netif_stop_queue(netdev);
> if (!test_bit(RTL8150_UNPLUG, &dev->flags))
> disable_net_traffic(dev);
> unlink_all_urbs(dev);
>
> - return res;
> + return 0;
> }
>
> static void rtl8150_get_drvinfo(struct net_device *netdev, struct ethtool_drvinfo *info)
> --
> 1.8.1.2
>
^ permalink raw reply
* [PATCH net] net/mlx4_en: Add VXLAN ndo calls to the PF net device ops too
From: Or Gerlitz @ 2014-11-18 15:51 UTC (permalink / raw)
To: David S. Miller
Cc: netdev, Florian Westphal, Amir Vadai, Saeed Mahameed, Or Gerlitz
This is currently missing, which results in a crash when one attempts
to set VXLAN tunnel over the mlx4_en when acting as PF.
[ 2408.785472] BUG: unable to handle kernel NULL pointer dereference at (null)
[...]
[ 2408.994104] Call Trace:
[ 2408.996584] [<ffffffffa021f7f5>] ? vxlan_get_rx_port+0xd6/0x103 [vxlan]
[ 2409.003316] [<ffffffffa021f71f>] ? vxlan_lowerdev_event+0xf2/0xf2 [vxlan]
[ 2409.010225] [<ffffffffa0630358>] mlx4_en_start_port+0x862/0x96a [mlx4_en]
[ 2409.017132] [<ffffffffa063070f>] mlx4_en_open+0x17f/0x1b8 [mlx4_en]
While here, make sure to invoke vxlan_get_rx_port() only when VXLAN
offloads are actually enabled and not when they are only supported.
Reported-by: Ido Shamay <idos@mellanox.com>
Signed-off-by: Or Gerlitz <ogerlitz@mellanox.com>
---
drivers/net/ethernet/mellanox/mlx4/en_netdev.c | 7 ++++++-
1 files changed, 6 insertions(+), 1 deletions(-)
diff --git a/drivers/net/ethernet/mellanox/mlx4/en_netdev.c b/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
index c5fcc56..4d69e38 100644
--- a/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
+++ b/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
@@ -1693,7 +1693,7 @@ int mlx4_en_start_port(struct net_device *dev)
mlx4_set_stats_bitmap(mdev->dev, &priv->stats_bitmap);
#ifdef CONFIG_MLX4_EN_VXLAN
- if (priv->mdev->dev->caps.flags2 & MLX4_DEV_CAP_FLAG2_VXLAN_OFFLOADS)
+ if (priv->mdev->dev->caps.tunnel_offload_mode == MLX4_TUNNEL_OFFLOAD_MODE_VXLAN)
vxlan_get_rx_port(dev);
#endif
priv->port_up = true;
@@ -2422,6 +2422,11 @@ static const struct net_device_ops mlx4_netdev_ops_master = {
.ndo_rx_flow_steer = mlx4_en_filter_rfs,
#endif
.ndo_get_phys_port_id = mlx4_en_get_phys_port_id,
+#ifdef CONFIG_MLX4_EN_VXLAN
+ .ndo_add_vxlan_port = mlx4_en_add_vxlan_port,
+ .ndo_del_vxlan_port = mlx4_en_del_vxlan_port,
+ .ndo_gso_check = mlx4_en_gso_check,
+#endif
};
int mlx4_en_init_netdev(struct mlx4_en_dev *mdev, int port,
--
1.7.1
^ permalink raw reply related
* Re: [PATCH 0/4] Add lightweight memory barriers fast_rmb() and fast_wmb()
From: Alexander Duyck @ 2014-11-18 15:44 UTC (permalink / raw)
To: David Laight, linux-arch@vger.kernel.org, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org
Cc: mathieu.desnoyers@polymtl.ca, peterz@infradead.org,
benh@kernel.crashing.org, heiko.carstens@de.ibm.com,
mingo@kernel.org, mikey@neuling.org, linux@arm.linux.org.uk,
donald.c.skidmore@intel.com, matthew.vick@intel.com,
geert@linux-m68k.org, jeffrey.t.kirsher@intel.com,
romieu@fr.zoreil.com, paulmck@linux.vnet.ibm.com,
nic_swsd@realtek.com, will.deacon@arm.com, michael@ellerman.id.au,
tony.luck@intel.com, "torvalds@li
In-Reply-To: <063D6719AE5E284EB5DD2968C1650D6D1C9F2930@AcuExch.aculab.com>
On 11/18/2014 01:57 AM, David Laight wrote:
> From: Alexander Duyck
>> These patches introduce two new primitives for synchronizing cache-enabled
>> memory writes and reads. These two new primitives are:
>>
>> fast_rmb()
>> fast_wmb()
> Not sure I like the names.
> If the aim is to sync data into the local cache so that hardware
> that is doing cache-snooping accesses sees the data then maybe
> local_rmb() and local_wmb()
Yeah, that is the general consensus. I am planning to change them to
coherent_rmb() and coherent_wmb().
> IIRC read_barrier_depends() is a nop on everything except alpha.
> Maybe add the default if it isn't defined by the MD file?
>
> David
>
From my patch the only two I saw define it were alpha and blackfin. It
is already defined in asm-generic, the rest is just clean-up since I
suspect some of the arch tree barrier.h calls just borrowed from
asm-generic without sorting out what became redundancies.
Thanks,
Alex
^ permalink raw reply
* Re: [PATCH net-next] PPC: bpf_jit_comp: Unify BPF_MOD | BPF_X and BPF_DIV | BPF_X
From: Alexei Starovoitov @ 2014-11-18 15:37 UTC (permalink / raw)
To: Denis Kirjanov
Cc: Michael Ellerman, netdev@vger.kernel.org, Philippe Bergheaud,
linuxppc-dev, Daniel Borkmann
In-Reply-To: <CAOJe8K0_u51QkKmVoVmFuGaDtKfcHP6tSWx_fgJM_erRRoQfUA@mail.gmail.com>
On Mon, Nov 17, 2014 at 10:58 PM, Denis Kirjanov <kda@linux-powerpc.org> wrote:
> Hi Michael,
>
> This patch added no new functionality so I haven't put the test
> results (of course I ran the test suite to check the patch).
>
> The output :
> [ 650.198958] test_bpf: Summary: 60 PASSED, 0 FAILED
Acked-by: Alexei Starovoitov <ast@plumgrid.com>
btw, please don't top post.
^ permalink raw reply
* Re: [PATCH net] bonding: fix curr_active_slave/carrier with loadbalance arp monitoring
From: Andy Gospodarek @ 2014-11-18 15:28 UTC (permalink / raw)
To: Veaceslav Falico
Cc: Nikolay Aleksandrov, netdev, davem, Jay Vosburgh, Andy Gospodarek,
Ding Tianhong
In-Reply-To: <20141118143727.GA2643@raspberrypi>
On Tue, Nov 18, 2014 at 03:37:27PM +0100, Veaceslav Falico wrote:
> On Tue, Nov 18, 2014 at 03:14:44PM +0100, Nikolay Aleksandrov wrote:
> >Since commit 6fde8f037e60 ("bonding: fix locking in
> >bond_loadbalance_arp_mon()") we can have a stale bond carrier state and
> >stale curr_active_slave when using arp monitoring in loadbalance modes. The
> >reason is that in bond_loadbalance_arp_mon() we can't have
> >do_failover == true but slave_state_changed == false, whenever do_failover
> >is true then slave_state_changed is also true. Then the following piece
> >from bond_loadbalance_arp_mon():
> > if (slave_state_changed) {
> > bond_slave_state_change(bond);
> > if (BOND_MODE(bond) == BOND_MODE_XOR)
> > bond_update_slave_arr(bond, NULL);
> > } else if (do_failover) {
>
> Ouch, must have been a big PITA to track :).
Agreed!
>
> > block_netpoll_tx();
> > bond_select_active_slave(bond);
> > unblock_netpoll_tx();
> > }
> >
> >will execute only the first branch, always and regardless of do_failover.
> >Since these two events aren't related in such way, we need to decouple and
> >consider them separately.
> >
> >For example this issue could lead to the following result:
> >Bonding Mode: load balancing (round-robin)
> >*MII Status: down*
> >MII Polling Interval (ms): 0
> >Up Delay (ms): 0
> >Down Delay (ms): 0
> >ARP Polling Interval (ms): 100
> >ARP IP target/s (n.n.n.n form): 192.168.9.2
> >
> >Slave Interface: ens12
> >*MII Status: up*
> >Speed: 10000 Mbps
> >Duplex: full
> >Link Failure Count: 2
> >Permanent HW addr: 00:0f:53:01:42:2c
> >Slave queue ID: 0
> >
> >Slave Interface: eth1
> >*MII Status: up*
> >Speed: Unknown
> >Duplex: Unknown
> >Link Failure Count: 70
> >Permanent HW addr: 52:54:00:2f:0f:8e
> >Slave queue ID: 0
> >
> >Since some interfaces are up, then the status of the bond should also be
> >up, but it will never change unless something invokes bond_set_carrier()
> >(i.e. enslave, bond_select_active_slave etc). Now, if I force the
> >calling of bond_select_active_slave via for example changing
> >primary_reselect (it can change in any mode), then the MII status goes to
> >"up" because it calls bond_select_active_slave() which should've been done
> >from bond_loadbalance_arp_mon() itself.
> >
> >CC: Veaceslav Falico <vfalico@gmail.com>
>
> Acked-by: Veaceslav Falico <vfalico@gmail.com>
Acked-by: Andy Gospodarek <gospo@cumulusnetworks.com>
>
> >CC: Jay Vosburgh <j.vosburgh@gmail.com>
> >CC: Andy Gospodarek <andy@greyhouse.net>
> >CC: Ding Tianhong <dingtianhong@huawei.com>
> >
> >Fixes: 6fde8f037e60 ("bonding: fix locking in bond_loadbalance_arp_mon()")
> >Signed-off-by: Nikolay Aleksandrov <nikolay@redhat.com>
> >---
> >Note: I left the parent if() the same even though we can shorten it. I think
> > it's better this way since it shows that any of the two events can cause
> > it to enter even though currently we can't have do_failover without
> > slave_state_changed, that may also change in the future.
> >
> >drivers/net/bonding/bond_main.c | 3 ++-
> >1 file changed, 2 insertions(+), 1 deletion(-)
> >
> >diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
> >index c9ac06cfe6b7..a5115fb7cf33 100644
> >--- a/drivers/net/bonding/bond_main.c
> >+++ b/drivers/net/bonding/bond_main.c
> >@@ -2471,7 +2471,8 @@ static void bond_loadbalance_arp_mon(struct work_struct *work)
> > bond_slave_state_change(bond);
> > if (BOND_MODE(bond) == BOND_MODE_XOR)
> > bond_update_slave_arr(bond, NULL);
> >- } else if (do_failover) {
> >+ }
> >+ if (do_failover) {
> > block_netpoll_tx();
> > bond_select_active_slave(bond);
> > unblock_netpoll_tx();
> >--
> >1.9.3
> >
> --
> 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: [PATCH net] bonding: fix curr_active_slave/carrier with loadbalance arp monitoring
From: Veaceslav Falico @ 2014-11-18 14:37 UTC (permalink / raw)
To: Nikolay Aleksandrov
Cc: netdev, davem, Jay Vosburgh, Andy Gospodarek, Ding Tianhong
In-Reply-To: <1416320084-25339-1-git-send-email-nikolay@redhat.com>
On Tue, Nov 18, 2014 at 03:14:44PM +0100, Nikolay Aleksandrov wrote:
>Since commit 6fde8f037e60 ("bonding: fix locking in
>bond_loadbalance_arp_mon()") we can have a stale bond carrier state and
>stale curr_active_slave when using arp monitoring in loadbalance modes. The
>reason is that in bond_loadbalance_arp_mon() we can't have
>do_failover == true but slave_state_changed == false, whenever do_failover
>is true then slave_state_changed is also true. Then the following piece
>from bond_loadbalance_arp_mon():
> if (slave_state_changed) {
> bond_slave_state_change(bond);
> if (BOND_MODE(bond) == BOND_MODE_XOR)
> bond_update_slave_arr(bond, NULL);
> } else if (do_failover) {
Ouch, must have been a big PITA to track :).
> block_netpoll_tx();
> bond_select_active_slave(bond);
> unblock_netpoll_tx();
> }
>
>will execute only the first branch, always and regardless of do_failover.
>Since these two events aren't related in such way, we need to decouple and
>consider them separately.
>
>For example this issue could lead to the following result:
>Bonding Mode: load balancing (round-robin)
>*MII Status: down*
>MII Polling Interval (ms): 0
>Up Delay (ms): 0
>Down Delay (ms): 0
>ARP Polling Interval (ms): 100
>ARP IP target/s (n.n.n.n form): 192.168.9.2
>
>Slave Interface: ens12
>*MII Status: up*
>Speed: 10000 Mbps
>Duplex: full
>Link Failure Count: 2
>Permanent HW addr: 00:0f:53:01:42:2c
>Slave queue ID: 0
>
>Slave Interface: eth1
>*MII Status: up*
>Speed: Unknown
>Duplex: Unknown
>Link Failure Count: 70
>Permanent HW addr: 52:54:00:2f:0f:8e
>Slave queue ID: 0
>
>Since some interfaces are up, then the status of the bond should also be
>up, but it will never change unless something invokes bond_set_carrier()
>(i.e. enslave, bond_select_active_slave etc). Now, if I force the
>calling of bond_select_active_slave via for example changing
>primary_reselect (it can change in any mode), then the MII status goes to
>"up" because it calls bond_select_active_slave() which should've been done
>from bond_loadbalance_arp_mon() itself.
>
>CC: Veaceslav Falico <vfalico@gmail.com>
Acked-by: Veaceslav Falico <vfalico@gmail.com>
>CC: Jay Vosburgh <j.vosburgh@gmail.com>
>CC: Andy Gospodarek <andy@greyhouse.net>
>CC: Ding Tianhong <dingtianhong@huawei.com>
>
>Fixes: 6fde8f037e60 ("bonding: fix locking in bond_loadbalance_arp_mon()")
>Signed-off-by: Nikolay Aleksandrov <nikolay@redhat.com>
>---
>Note: I left the parent if() the same even though we can shorten it. I think
> it's better this way since it shows that any of the two events can cause
> it to enter even though currently we can't have do_failover without
> slave_state_changed, that may also change in the future.
>
> drivers/net/bonding/bond_main.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
>diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
>index c9ac06cfe6b7..a5115fb7cf33 100644
>--- a/drivers/net/bonding/bond_main.c
>+++ b/drivers/net/bonding/bond_main.c
>@@ -2471,7 +2471,8 @@ static void bond_loadbalance_arp_mon(struct work_struct *work)
> bond_slave_state_change(bond);
> if (BOND_MODE(bond) == BOND_MODE_XOR)
> bond_update_slave_arr(bond, NULL);
>- } else if (do_failover) {
>+ }
>+ if (do_failover) {
> block_netpoll_tx();
> bond_select_active_slave(bond);
> unblock_netpoll_tx();
>--
>1.9.3
>
^ permalink raw reply
* Re: [net-next 03/12] i40e: Handle a single mss packet with more than 8 frags
From: Eric Dumazet @ 2014-11-18 14:33 UTC (permalink / raw)
To: David Laight
Cc: 'Nelson, Shannon', Kirsher, Jeffrey T,
davem@davemloft.net, Kong, Serey, netdev@vger.kernel.org,
nhorman@redhat.com, sassmann@redhat.com, jogreene@redhat.com
In-Reply-To: <063D6719AE5E284EB5DD2968C1650D6D1C9F290B@AcuExch.aculab.com>
On Tue, 2014-11-18 at 09:46 +0000, David Laight wrote:
> That isn't entirely clear from the patch context.
gso_segs = 1 was not clear ???
gso_segs = 1;
if (skb_shinfo(skb)->nr_frags >= I40E_MAX_BUFFER_TXD)
skb_linearize(skb);
Nelson, what happens if a packet has 2 segments and uses 17 fragments ?
Is the hardware OK with that ?
netperf -t TCP_SENDFILE -- -m 171
I did the test and it seems fine on my host, but maybe it was pure luck.
While the following indeed froze the card :
netperf -H 7.7.7.27 -t TCP_SENDFILE -- -m 30
[3257268.702689] i40e 0000:84:00.0: requesting a pf reset
[3257268.831786] i40e 0000:84:00.0: i40e_ptp_init: added PHC on eth2
[3257268.912798] i40e 0000:84:00.0 eth2: NIC Link is Up 40 Gbps Full Duplex, Flow Control: RX/TX
[3257338.895753] i40e 0000:84:00.0: Detected Tx Unit Hang
[3257338.895753] VSI <518>
[3257338.895753] Tx Queue <16>
[3257338.895753] next_to_use <17c>
[3257338.895753] next_to_clean <b2>
[3257338.895757] i40e 0000:84:00.0: tx_bi[next_to_clean]
[3257338.895757] time_stamp <1c20f09c9>
[3257338.895757] jiffies <1c20f0eac>
[3257338.895759] i40e 0000:84:00.0: tx hang detected on queue 16, resetting adapter
[3257338.895769] i40e 0000:84:00.0 eth2: tx_timeout recovery level 1
[3257338.895926] i40e 0000:84:00.0: i40e_vsi_control_tx: VSI seid 518 Tx ring 0 disable timeout
[3257338.949922] i40e 0000:84:00.0: i40e_vsi_control_tx: VSI seid 520 Tx ring 64 disable timeout
[3257339.410705] i40e 0000:84:00.0: PF reset failed, -15
>
> For a non-TSO packet the skb_serialize() is less likely to fail
> since it doesn't need contiguous pages.
Any memory allocation can fail, regardless of the size.
We do not want a memory stress being able to crash the host,
likely or not. It is not worth discussing this, really.
^ permalink raw reply
* [PATCH net] bonding: fix curr_active_slave/carrier with loadbalance arp monitoring
From: Nikolay Aleksandrov @ 2014-11-18 14:14 UTC (permalink / raw)
To: netdev
Cc: davem, Nikolay Aleksandrov, Veaceslav Falico, Jay Vosburgh,
Andy Gospodarek, Ding Tianhong
Since commit 6fde8f037e60 ("bonding: fix locking in
bond_loadbalance_arp_mon()") we can have a stale bond carrier state and
stale curr_active_slave when using arp monitoring in loadbalance modes. The
reason is that in bond_loadbalance_arp_mon() we can't have
do_failover == true but slave_state_changed == false, whenever do_failover
is true then slave_state_changed is also true. Then the following piece
from bond_loadbalance_arp_mon():
if (slave_state_changed) {
bond_slave_state_change(bond);
if (BOND_MODE(bond) == BOND_MODE_XOR)
bond_update_slave_arr(bond, NULL);
} else if (do_failover) {
block_netpoll_tx();
bond_select_active_slave(bond);
unblock_netpoll_tx();
}
will execute only the first branch, always and regardless of do_failover.
Since these two events aren't related in such way, we need to decouple and
consider them separately.
For example this issue could lead to the following result:
Bonding Mode: load balancing (round-robin)
*MII Status: down*
MII Polling Interval (ms): 0
Up Delay (ms): 0
Down Delay (ms): 0
ARP Polling Interval (ms): 100
ARP IP target/s (n.n.n.n form): 192.168.9.2
Slave Interface: ens12
*MII Status: up*
Speed: 10000 Mbps
Duplex: full
Link Failure Count: 2
Permanent HW addr: 00:0f:53:01:42:2c
Slave queue ID: 0
Slave Interface: eth1
*MII Status: up*
Speed: Unknown
Duplex: Unknown
Link Failure Count: 70
Permanent HW addr: 52:54:00:2f:0f:8e
Slave queue ID: 0
Since some interfaces are up, then the status of the bond should also be
up, but it will never change unless something invokes bond_set_carrier()
(i.e. enslave, bond_select_active_slave etc). Now, if I force the
calling of bond_select_active_slave via for example changing
primary_reselect (it can change in any mode), then the MII status goes to
"up" because it calls bond_select_active_slave() which should've been done
from bond_loadbalance_arp_mon() itself.
CC: Veaceslav Falico <vfalico@gmail.com>
CC: Jay Vosburgh <j.vosburgh@gmail.com>
CC: Andy Gospodarek <andy@greyhouse.net>
CC: Ding Tianhong <dingtianhong@huawei.com>
Fixes: 6fde8f037e60 ("bonding: fix locking in bond_loadbalance_arp_mon()")
Signed-off-by: Nikolay Aleksandrov <nikolay@redhat.com>
---
Note: I left the parent if() the same even though we can shorten it. I think
it's better this way since it shows that any of the two events can cause
it to enter even though currently we can't have do_failover without
slave_state_changed, that may also change in the future.
drivers/net/bonding/bond_main.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index c9ac06cfe6b7..a5115fb7cf33 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -2471,7 +2471,8 @@ static void bond_loadbalance_arp_mon(struct work_struct *work)
bond_slave_state_change(bond);
if (BOND_MODE(bond) == BOND_MODE_XOR)
bond_update_slave_arr(bond, NULL);
- } else if (do_failover) {
+ }
+ if (do_failover) {
block_netpoll_tx();
bond_select_active_slave(bond);
unblock_netpoll_tx();
--
1.9.3
^ permalink raw reply related
* Re: Query about Dynamic Right Sizing implementation in linux-kernel
From: Eric Dumazet @ 2014-11-18 14:07 UTC (permalink / raw)
To: cprasad; +Cc: netdev
In-Reply-To: <400045440.131017.1416304451861.JavaMail.root@mail.cse.iitm.ac.in>
On Tue, 2014-11-18 at 15:24 +0530, cprasad@cse.iitm.ac.in wrote:
> Hi,
> I found a DRS patch for verison 2.4.8 in lanl.gov page and I have no
> idea if it is incorporated into the kernel. So I would like to know if
> Dynamic Right Sizing(DRS) is implemented in kernel.If so from which
> version of kernel was it implemented?
It is implemented yes.
Last patch we did about that was for linux-3.13
http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=b0983d3c9b132c33b6fb2e28d157a1edc18a173c
^ permalink raw reply
* [PATCH 2/2] net: can: comparison of unsigned variable
From: Sudip Mukherjee @ 2014-11-18 13:47 UTC (permalink / raw)
To: Wolfgang Grandegger, Marc Kleine-Budde, Michal Simek,
Sören Brinkmann
Cc: Sudip Mukherjee, linux-can, netdev, linux-arm-kernel,
linux-kernel
In-Reply-To: <1416318427-28676-1-git-send-email-sudipm.mukherjee@gmail.com>
err was of the type u32. it was being compared with < 0, and being
an unsigned variable the comparison would have been always false.
moreover, err was getting the return value from set_reset_mode()
and xcan_set_bittiming(), and both are returning int.
Signed-off-by: Sudip Mukherjee <sudip@vectorindia.org>
---
drivers/net/can/xilinx_can.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/net/can/xilinx_can.c b/drivers/net/can/xilinx_can.c
index 72fe96f..67c2dd4 100644
--- a/drivers/net/can/xilinx_can.c
+++ b/drivers/net/can/xilinx_can.c
@@ -300,7 +300,8 @@ static int xcan_set_bittiming(struct net_device *ndev)
static int xcan_chip_start(struct net_device *ndev)
{
struct xcan_priv *priv = netdev_priv(ndev);
- u32 err, reg_msr, reg_sr_mask;
+ u32 reg_msr, reg_sr_mask;
+ int err;
unsigned long timeout;
/* Check if it is in reset mode */
--
1.8.1.2
^ permalink raw reply related
* [PATCH 1/2] net: can: remove unused variable
From: Sudip Mukherjee @ 2014-11-18 13:47 UTC (permalink / raw)
To: Wolfgang Grandegger, Marc Kleine-Budde
Cc: Sudip Mukherjee, linux-can, netdev, linux-kernel
these variable were only assigned some values, but then never
reused again.
so they are safe to be removed.
Signed-off-by: Sudip Mukherjee <sudip@vectorindia.org>
---
drivers/net/can/sja1000/kvaser_pci.c | 5 +----
drivers/net/can/usb/ems_usb.c | 3 +--
drivers/net/can/usb/esd_usb2.c | 2 --
3 files changed, 2 insertions(+), 8 deletions(-)
diff --git a/drivers/net/can/sja1000/kvaser_pci.c b/drivers/net/can/sja1000/kvaser_pci.c
index 8ff3424..15c00fa 100644
--- a/drivers/net/can/sja1000/kvaser_pci.c
+++ b/drivers/net/can/sja1000/kvaser_pci.c
@@ -214,7 +214,7 @@ static int kvaser_pci_add_chan(struct pci_dev *pdev, int channel,
struct net_device *dev;
struct sja1000_priv *priv;
struct kvaser_pci *board;
- int err, init_step;
+ int err;
dev = alloc_sja1000dev(sizeof(struct kvaser_pci));
if (dev == NULL)
@@ -235,7 +235,6 @@ static int kvaser_pci_add_chan(struct pci_dev *pdev, int channel,
if (channel == 0) {
board->xilinx_ver =
ioread8(board->res_addr + XILINX_VERINT) >> 4;
- init_step = 2;
/* Assert PTADR# - we're in passive mode so the other bits are
not important */
@@ -264,8 +263,6 @@ static int kvaser_pci_add_chan(struct pci_dev *pdev, int channel,
priv->irq_flags = IRQF_SHARED;
dev->irq = pdev->irq;
- init_step = 4;
-
dev_info(&pdev->dev, "reg_base=%p conf_addr=%p irq=%d\n",
priv->reg_base, board->conf_addr, dev->irq);
diff --git a/drivers/net/can/usb/ems_usb.c b/drivers/net/can/usb/ems_usb.c
index 00f2534..29d3f09 100644
--- a/drivers/net/can/usb/ems_usb.c
+++ b/drivers/net/can/usb/ems_usb.c
@@ -434,10 +434,9 @@ static void ems_usb_read_bulk_callback(struct urb *urb)
if (urb->actual_length > CPC_HEADER_SIZE) {
struct ems_cpc_msg *msg;
u8 *ibuf = urb->transfer_buffer;
- u8 msg_count, again, start;
+ u8 msg_count, start;
msg_count = ibuf[0] & ~0x80;
- again = ibuf[0] & 0x80;
start = CPC_HEADER_SIZE;
diff --git a/drivers/net/can/usb/esd_usb2.c b/drivers/net/can/usb/esd_usb2.c
index b7c9e8b..d25e88f 100644
--- a/drivers/net/can/usb/esd_usb2.c
+++ b/drivers/net/can/usb/esd_usb2.c
@@ -464,7 +464,6 @@ static void esd_usb2_write_bulk_callback(struct urb *urb)
{
struct esd_tx_urb_context *context = urb->context;
struct esd_usb2_net_priv *priv;
- struct esd_usb2 *dev;
struct net_device *netdev;
size_t size = sizeof(struct esd_usb2_msg);
@@ -472,7 +471,6 @@ static void esd_usb2_write_bulk_callback(struct urb *urb)
priv = context->priv;
netdev = priv->netdev;
- dev = priv->usb2;
/* free up our allocated buffer */
usb_free_coherent(urb->dev, size,
--
1.8.1.2
^ permalink raw reply related
* Re: ipx: fix locking regression in ipx_sendmsg and ipx_recvmsg
From: Arnd Bergmann @ 2014-11-18 13:37 UTC (permalink / raw)
To: Jiri Bohac; +Cc: Arnaldo Carvalho de Melo, netdev
In-Reply-To: <20141117013448.GA26743@midget.suse.cz>
On Monday 17 November 2014 02:34:48 Jiri Bohac wrote:
> This fixes an old regression introduced by commit
> b0d0d915 (ipx: remove the BKL).
>
> When a recvmsg syscall blocks waiting for new data, no data can be sent on the
> same socket with sendmsg because ipx_recvmsg() sleeps with the socket locked.
>
> This breaks mars-nwe (NetWare emulator):
> - the ncpserv process reads the request using recvmsg
> - ncpserv forks and spawns nwconn
> - ncpserv calls a (blocking) recvmsg and waits for new requests
> - nwconn deadlocks in sendmsg on the same socket
>
> Commit b0d0d915 has simply replaced BKL locking with
> lock_sock/release_sock. Unlike now, BKL got unlocked while
> sleeping, so a blocking recvmsg did not block a concurrent
> sendmsg.
>
> Similarly, a potentially sleeping sendmsg() could block calls to recvmsg().
>
> Only keep the socket locked while actually working with the socket data and
> release it prior to calling skb_recv_datagram() / ipxitf_send().
>
>
> Signed-off-by: Jiri Bohac <jbohac@suse.cz>
Hi Jiri,
I'm very sorry about the regression my patch introduced, glad you worked
it out. Your patch looks correct to me, but I suspect we can do it in
a simpler way, based on what I found I did in the respective appletalk
and x25 BKL removal patches. From all I can tell, those do not have
the same problem, which is a relief to me.
Some questions:
> @@ -1745,12 +1745,16 @@ static int ipx_sendmsg(struct kiocb *iocb, struct socket *sock,
> memcpy(usipx->sipx_node, ipxs->dest_addr.node, IPX_NODE_LEN);
> }
>
> + /* releases sk */
> rc = ipxrtr_route_packet(sk, usipx, msg->msg_iov, len,
> flags & MSG_DONTWAIT);
> if (rc >= 0)
> rc = len;
> -out:
> + goto out;
> +
> +out_release:
> release_sock(sk);
> +out:
> return rc;
> }
>
Does ipxrtr_route_packet() actually sleep while waiting for the network,
or is it possible that you only need to change the recvmsg path?
If you need to change this function, have you considered doing it
one of these two ways:
a) only change the ipxrtr_route_packet function to release the lock
before sleeping and then reaquiring it but not change ipx_sendmsg
b) figure out whether ipx_sendmsg actually relies on the lock at all,
and if it doesn't then remove the locking, or limit the scope to
the parts that do.
> @@ -1776,20 +1780,21 @@ static int ipx_recvmsg(struct kiocb *iocb, struct socket *sock,
> #ifdef CONFIG_IPX_INTERN
> rc = -ENETDOWN;
> if (!ipxs->intrfc)
> - goto out; /* Someone zonked the iface */
> + goto out_release; /* Someone zonked the iface */
> memcpy(uaddr.sipx_node, ipxs->intrfc->if_node, IPX_NODE_LEN);
> #endif /* CONFIG_IPX_INTERN */
>
> rc = __ipx_bind(sock, (struct sockaddr *)&uaddr,
> sizeof(struct sockaddr_ipx));
> if (rc)
> - goto out;
> + goto out_release;
> }
>
> rc = -ENOTCONN;
> if (sock_flag(sk, SOCK_ZAPPED))
> - goto out;
> + goto out_release;
>
> + release_sock(sk);
> skb = skb_recv_datagram(sk, flags & ~MSG_DONTWAIT,
> flags & MSG_DONTWAIT, &rc);
> if (!skb) {
Same thing here: I think your patch could be simplified if you just
release the socket lock before calling skb_recv_datagram and get
it back afterwards, and it would be much simpler if you could
show that the lock is not needed at all.
Arnd
^ permalink raw reply
* [PATCH] usbnet: rtl8150: remove unused variable
From: Sudip Mukherjee @ 2014-11-18 12:59 UTC (permalink / raw)
To: Petko Manolov; +Cc: Sudip Mukherjee, linux-usb, netdev, linux-kernel
we were just returning the initial value of res, instead now
we are returning the value directly.
Signed-off-by: Sudip Mukherjee <sudip@vectorindia.org>
---
drivers/net/usb/rtl8150.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/net/usb/rtl8150.c b/drivers/net/usb/rtl8150.c
index 6e87e57..d37b7dc 100644
--- a/drivers/net/usb/rtl8150.c
+++ b/drivers/net/usb/rtl8150.c
@@ -753,14 +753,13 @@ static int rtl8150_open(struct net_device *netdev)
static int rtl8150_close(struct net_device *netdev)
{
rtl8150_t *dev = netdev_priv(netdev);
- int res = 0;
netif_stop_queue(netdev);
if (!test_bit(RTL8150_UNPLUG, &dev->flags))
disable_net_traffic(dev);
unlink_all_urbs(dev);
- return res;
+ return 0;
}
static void rtl8150_get_drvinfo(struct net_device *netdev, struct ethtool_drvinfo *info)
--
1.8.1.2
^ permalink raw reply related
* Re: [PATCH 2/4] arch: Add lightweight memory barriers fast_rmb() and fast_wmb()
From: Will Deacon @ 2014-11-18 11:58 UTC (permalink / raw)
To: Alexander Duyck
Cc: Benjamin Herrenschmidt, Alexander Duyck,
linux-arch@vger.kernel.org, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org, mathieu.desnoyers@polymtl.ca,
peterz@infradead.org, heiko.carstens@de.ibm.com, mingo@kernel.org,
mikey@neuling.org, linux@arm.linux.org.uk,
donald.c.skidmore@intel.com, matthew.vick@intel.com,
geert@linux-m68k.org, jeffrey.t.kirsher@intel.com,
romieu@fr.zoreil.com, paulmck@linux.vnet.ibm.com
In-Reply-To: <546AB959.1020602@redhat.com>
On Tue, Nov 18, 2014 at 03:13:29AM +0000, Alexander Duyck wrote:
> On 11/17/2014 04:39 PM, Benjamin Herrenschmidt wrote:
> > On Mon, 2014-11-17 at 12:24 -0800, Alexander Duyck wrote:
> >> Yes and no. So for example on ARM I used the dmb() operation, however
> >> I
> >> have to use the barrier at the system level instead of just the inner
> >> shared domain. However on many other architectures they are just the
> >> same as the smp_* variants.
> >>
> >> Basically the resultant code is somewhere between the smp and non-smp
> >> barriers in terms of what they cover.
> > There I don't quite follow you. You need to explain better especially in
> > the documentation because otherwise people will get it wrong...
> >
> > If it's ordering in the coherent domain, I fail to see how a DMA agent
> > is different than another processor when it comes to barriers, so I fail
> > to see the difference with smp_*
> >
> > I understand the MMIO vs. memory issue, we do have the same on powerpc,
> > but that other aspect eludes me.
> >
>
> ARM adds some funky things. They have two different types of
> primitives, a dmb() which is a data memory barrier, and a dsb() which is
> a data synchronization barrier. Then with each of those they have the
> "domains" the barriers are effective within.
>
> So for example on ARM a rmb() is dsb(sy) which means it is a system wide
> synchronization barrier which stops execution on the CPU core until the
> read completes. However the smp_rmb() is a dmb(ish) which means it is
> only a barrier as far as the inner shareable domain which I believe only
> goes as far as the local shared cache hierarchy and only guarantees read
> ordering without necessarily halting the CPU or stopping in-order
> speculative reads. So what a coherent_rmb() would be in my setup is
> dmb(sy) which means the barrier runs all the way out to memory, and it
> is allowed to speculative read as long as it does it in order.
>
> If it is still unclear you might check out Will Deacon's talk on the
> topic at https://www.youtube.com/watch?v=6ORn6_35kKo, at about 7:00 in
> he explains the whole domains thing, and at 13:30 he explains dmb()/dsb().
So actually, this is an interesting case where the barrier would like to
know whether the memory returned by dma_alloc_coherent is h/w coherent
(normal, cacheable) or s/w coherent (normal, non-cacheable). I think Ben
is thinking of the h/w coherent case (i.e. actual snooping into the CPU
caches by the DMA master).
For the former, we could use inner-shareable barriers. For the latter, we'd
need to use outer-shareable barriers.
If we can't tell, then these should be dmb(osh), which will work for both.
Will
^ 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