* [PATCH net-next] net: better IFF_XMIT_DST_RELEASE support
From: Eric Dumazet @ 2014-10-02 16:34 UTC (permalink / raw)
To: David Miller; +Cc: netdev
From: Eric Dumazet <edumazet@google.com>
Testing xmit_more support with netperf and connected UDP sockets,
I found strange dst refcount false sharing.
Current handling of IFF_XMIT_DST_RELEASE is not optimal.
dropping dst in validate_xmit_skb() is certainly too late in case
packet was queued by cpu X but dequeued by cpu Y
The logical point to take care of drop/force is in __dev_queue_xmit()
before even taking qdisc lock.
Signed-off-by: Eric Dumazet <edumazet@google.com>
---
include/linux/netdevice.h | 3 +--
net/core/dev.c | 16 ++++++++--------
2 files changed, 9 insertions(+), 10 deletions(-)
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 9b7fbacb6296..ea8b23510c9e 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -1168,8 +1168,7 @@ struct net_device_ops {
* @IFF_ISATAP: ISATAP interface (RFC4214)
* @IFF_MASTER_ARPMON: bonding master, ARP mon in use
* @IFF_WAN_HDLC: WAN HDLC device
- * @IFF_XMIT_DST_RELEASE: dev_hard_start_xmit() is allowed to
- * release skb->dst
+ * @IFF_XMIT_DST_RELEASE: dev_queue_xmit() is allowed to release skb->dst
* @IFF_DONT_BRIDGE: disallow bridging this ether dev
* @IFF_DISABLE_NETPOLL: disable netpoll at run-time
* @IFF_MACVLAN_PORT: device used as macvlan port
diff --git a/net/core/dev.c b/net/core/dev.c
index e55c546717d4..e178b16b2e53 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -2662,11 +2662,6 @@ struct sk_buff *validate_xmit_skb(struct sk_buff *skb, struct net_device *dev)
if (skb->next)
return skb;
- /* If device doesn't need skb->dst, release it right now while
- * its hot in this cpu cache
- */
- if (dev->priv_flags & IFF_XMIT_DST_RELEASE)
- skb_dst_drop(skb);
features = netif_skb_features(skb);
skb = validate_xmit_vlan(skb, features);
@@ -2781,8 +2776,6 @@ static inline int __dev_xmit_skb(struct sk_buff *skb, struct Qdisc *q,
* waiting to be sent out; and the qdisc is not running -
* xmit the skb directly.
*/
- if (!(dev->priv_flags & IFF_XMIT_DST_RELEASE))
- skb_dst_force(skb);
qdisc_bstats_update(q, skb);
@@ -2798,7 +2791,6 @@ static inline int __dev_xmit_skb(struct sk_buff *skb, struct Qdisc *q,
rc = NET_XMIT_SUCCESS;
} else {
- skb_dst_force(skb);
rc = q->enqueue(skb, q) & NET_XMIT_MASK;
if (qdisc_run_begin(q)) {
if (unlikely(contended)) {
@@ -2895,6 +2887,14 @@ static int __dev_queue_xmit(struct sk_buff *skb, void *accel_priv)
skb_update_prio(skb);
+ /* If device doesn't need skb->dst, release it right now while
+ * its hot in this cpu cache
+ */
+ if (dev->priv_flags & IFF_XMIT_DST_RELEASE)
+ skb_dst_drop(skb);
+ else
+ skb_dst_force(skb);
+
txq = netdev_pick_tx(dev, skb, accel_priv);
q = rcu_dereference_bh(txq->qdisc);
^ permalink raw reply related
* Re: [PATCH net-next] net: Cleanup skb cloning by adding SKB_FCLONE_FREE
From: Vijay Subramanian @ 2014-10-02 16:36 UTC (permalink / raw)
To: Eric Dumazet; +Cc: netdev, David Miller, Eric Dumazet
In-Reply-To: <1412249728.16704.76.camel@edumazet-glaptop2.roam.corp.google.com>
Sure. Will resend V2 shortly.
Vijay
On 2 October 2014 04:35, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> On Wed, 2014-10-01 at 23:33 -0700, Vijay Subramanian wrote:
>> SKB_FCLONE_UNAVAILABLE has overloaded meaning depending on type of skb.
>> 1: If skb is allocated from head_cache, it indicates fclone is not available.
>> 2: If skb is a companion fclone skb (allocated from fclone_cache), it indicates
>> it is available to be used.
>>
>> To avoid confusion for case 2 above, this patch replaces
>> SKB_FCLONE_UNAVAILABLE with SKB_FCLONE_FREE where appropriate. For fclone
>> companion skbs, this indicates it is free for use.
>>
>> SKB_FCLONE_UNAVAILABLE will now simply indicate skb is from head_cache and
>> cannot / will not have a companion fclone.
>>
>> Signed-off-by: Vijay Subramanian <subramanian.vijay@gmail.com>
>> ---
>> include/linux/skbuff.h | 3 ++-
>> net/core/skbuff.c | 8 ++++----
>> 2 files changed, 6 insertions(+), 5 deletions(-)
>>
>> diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
>> index 7c5036d..6c3fb9a 100644
>> --- a/include/linux/skbuff.h
>> +++ b/include/linux/skbuff.h
>> @@ -339,9 +339,10 @@ struct skb_shared_info {
>>
>>
>> enum {
>> - SKB_FCLONE_UNAVAILABLE,
>> + SKB_FCLONE_UNAVAILABLE, /* skb has no fclone */
>> SKB_FCLONE_ORIG,
>> SKB_FCLONE_CLONE,
>> + SKB_FCLONE_FREE, /* this fclone skb is available */
>> };
>>
> Please comment all the states, now there is no ambiguity ?
>
>
>
^ permalink raw reply
* [PATCH] net: systemport: fix bcm_sysport_insert_tsb()
From: Florian Fainelli @ 2014-10-02 16:43 UTC (permalink / raw)
To: netdev; +Cc: davem, Florian Fainelli
Similar to commit bc23333ba11fb7f959b7e87e121122f5a0fbbca8 ("net:
bcmgenet: fix bcmgenet_put_tx_csum()"), we need to return the skb
pointer in case we had to reallocate the SKB headroom.
Fixes: 80105befdb4b8 ("net: systemport: add Broadcom SYSTEMPORT Ethernet MAC driver")
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
drivers/net/ethernet/broadcom/bcmsysport.c | 13 +++++++------
1 file changed, 7 insertions(+), 6 deletions(-)
diff --git a/drivers/net/ethernet/broadcom/bcmsysport.c b/drivers/net/ethernet/broadcom/bcmsysport.c
index 77f1ff7396ac..075688188644 100644
--- a/drivers/net/ethernet/broadcom/bcmsysport.c
+++ b/drivers/net/ethernet/broadcom/bcmsysport.c
@@ -857,7 +857,8 @@ static irqreturn_t bcm_sysport_wol_isr(int irq, void *dev_id)
return IRQ_HANDLED;
}
-static int bcm_sysport_insert_tsb(struct sk_buff *skb, struct net_device *dev)
+static struct sk_buff *bcm_sysport_insert_tsb(struct sk_buff *skb,
+ struct net_device *dev)
{
struct sk_buff *nskb;
struct bcm_tsb *tsb;
@@ -873,7 +874,7 @@ static int bcm_sysport_insert_tsb(struct sk_buff *skb, struct net_device *dev)
if (!nskb) {
dev->stats.tx_errors++;
dev->stats.tx_dropped++;
- return -ENOMEM;
+ return NULL;
}
skb = nskb;
}
@@ -892,7 +893,7 @@ static int bcm_sysport_insert_tsb(struct sk_buff *skb, struct net_device *dev)
ip_proto = ipv6_hdr(skb)->nexthdr;
break;
default:
- return 0;
+ return skb;
}
/* Get the checksum offset and the L4 (transport) offset */
@@ -911,7 +912,7 @@ static int bcm_sysport_insert_tsb(struct sk_buff *skb, struct net_device *dev)
tsb->l4_ptr_dest_map = csum_info;
}
- return 0;
+ return skb;
}
static netdev_tx_t bcm_sysport_xmit(struct sk_buff *skb,
@@ -945,8 +946,8 @@ static netdev_tx_t bcm_sysport_xmit(struct sk_buff *skb,
/* Insert TSB and checksum infos */
if (priv->tsb_en) {
- ret = bcm_sysport_insert_tsb(skb, dev);
- if (ret) {
+ skb = bcm_sysport_insert_tsb(skb, dev);
+ if (!skb) {
ret = NETDEV_TX_OK;
goto out;
}
--
1.9.1
^ permalink raw reply related
* Re: [PATCH] drivers/net/dsa/Kconfig: Let NET_DSA_BCM_SF2 depend on HAS_IOMEM
From: Florian Fainelli @ 2014-10-02 16:44 UTC (permalink / raw)
To: Chen Gang, davem, leitec, andrew
Cc: netdev@vger.kernel.org, Richard Weinberger,
linux-kernel@vger.kernel.org
In-Reply-To: <542D5DAC.4010001@gmail.com>
On 10/02/2014 07:14 AM, Chen Gang wrote:
> NET_DSA_BCM_SF2 need HAS_IOMEM, so depend on it, the related error (with
> allmodconfig under um):
>
> CC [M] drivers/net/dsa/bcm_sf2.o
> drivers/net/dsa/bcm_sf2.c: In function ‘bcm_sf2_sw_setup’:
> drivers/net/dsa/bcm_sf2.c:487:3: error: implicit declaration of function ‘iounmap’ [-Werror=implicit-function-declaration]
> iounmap(*base);
> ^
>
> Signed-off-by: Chen Gang <gang.chen.5i5j@gmail.com>
Acked-by: Florian Fainelli <f.fainelli@gmail.com>
> ---
> drivers/net/dsa/Kconfig | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/net/dsa/Kconfig b/drivers/net/dsa/Kconfig
> index ea0697e..9234d80 100644
> --- a/drivers/net/dsa/Kconfig
> +++ b/drivers/net/dsa/Kconfig
> @@ -47,6 +47,7 @@ config NET_DSA_MV88E6171
>
> config NET_DSA_BCM_SF2
> tristate "Broadcom Starfighter 2 Ethernet switch support"
> + depends on HAS_IOMEM
> select NET_DSA
> select NET_DSA_TAG_BRCM
> select FIXED_PHY if NET_DSA_BCM_SF2=y
>
^ permalink raw reply
* Re: [RFC PATCH linux 2/2] fs/proc: use a hash table for the directory entries
From: Stephen Hemminger @ 2014-10-02 16:46 UTC (permalink / raw)
To: Nicolas Dichtel
Cc: netdev, linux-kernel, davem, ebiederm, akpm, adobriyan, rui.xiang,
viro, oleg, gorcunov, kirill.shutemov, grant.likely, tytso,
Thierry Herbelot
In-Reply-To: <1412263501-6572-3-git-send-email-nicolas.dichtel@6wind.com>
On Thu, 2 Oct 2014 17:25:01 +0200
Nicolas Dichtel <nicolas.dichtel@6wind.com> wrote:
> From: Thierry Herbelot <thierry.herbelot@6wind.com>
>
> The current implementation for the directories in /proc is using a single
> linked list. This is slow when handling directories with large numbers of
> entries (eg netdevice-related entries when lots of tunnels are opened).
>
> This patch enables multiple linked lists. A hash based on the entry name is
> used to select the linked list for one given entry.
>
> The speed creation of netdevices is faster as shorter linked lists must be
> scanned when adding a new netdevice.
>
> Here are some numbers:
>
> dummy30000.batch contains 30 000 times 'link add type dummy'.
>
> Before the patch:
> time ip -b dummy30000.batch
> real 2m32.221s
> user 0m0.380s
> sys 2m30.610s
>
> After the patch:
> time ip -b dummy30000.batch
> real 1m57.190s
> user 0m0.350s
> sys 1m56.120s
>
> The single 'subdir' list head is replaced by a subdir hash table. The subdir
> hash buckets are only allocated for directories. The number of hash buckets
> is a compile-time parameter.
>
> For all functions which handle directory entries, an additional check on the
> directory nature of the dir entry ensures that pde_hash_buckets was allocated.
> This check was not needed as subdir was present for all dir entries, whether
> actual directories or simple files.
>
> Signed-off-by: Thierry Herbelot <thierry.herbelot@6wind.com>
> Signed-off-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>
I think the speed up is a good idea and makes sense.
It would be better to use exist hlist macros for hash table rather than
open coding it.
^ permalink raw reply
* Re: [net-next PATCH V6 0/2] qdisc: bulk dequeue support
From: Florian Westphal @ 2014-10-02 16:52 UTC (permalink / raw)
To: Tom Herbert
Cc: Jesper Dangaard Brouer, Linux Netdev List, David S. Miller,
Eric Dumazet, Hannes Frederic Sowa, Florian Westphal,
Daniel Borkmann, Jamal Hadi Salim, Alexander Duyck,
John Fastabend, Dave Taht, Toke Høiland-Jørgensen
In-Reply-To: <CA+mtBx8ecpd+S+E2o8A+Hzp3i2SO7XO0+F_DYA0ng+fjta7G5A@mail.gmail.com>
Tom Herbert <therbert@google.com> wrote:
> On Wed, Oct 1, 2014 at 1:35 PM, Jesper Dangaard Brouer
> <brouer@redhat.com> wrote:
> > This patchset uses DaveM's recent API changes to dev_hard_start_xmit(),
> > from the qdisc layer, to implement dequeue bulking.
> >
> > Patch01: "qdisc: bulk dequeue support for qdiscs with TCQ_F_ONETXQUEUE"
> > - Implement basic qdisc dequeue bulking
> > - This time, 100% relying on BQL limits, no magic safe-guard constants
> >
> > Patch02: "qdisc: dequeue bulking also pickup GSO/TSO packets"
> > - Extend bulking to bulk several GSO/TSO packets
> > - Seperate patch, as it introduce a small regression, see test section.
> >
> > We do have a patch03, which exports a userspace tunable as a BQL
> > tunable, that can byte-cap or disable the bulking/bursting. But we
> > could not agree on it internally, thus not sending it now. We
> > basically strive to avoid adding any new userspace tunable.
> >
> Unfortunately we probably still need something. If BQL were disabled
> (by setting BQL min_limit to infinity) then we'll always dequeue all
> the packets in the qdisc. Disabling BQL might be legitimate in
> deployment if say a bug is found in a device that prevents prompt
> transmit completions for some corner case.
Hmm. Thats confusing.
So you are saying to disable bql one should do
cat limit_max > limit_min ?
But thats not the same as having a bql-unaware driver.
Seems to get same behavior as non-bql aware driver (where
dql_avail always returns 0 since num_queued and adj_limit remain at 0)
is to set
echo 0 > limit_max
... which makes dql_avail() return 0, which then also turns off bulk
dequeue.
Confused,
Florian
^ permalink raw reply
* [PATCH V2 net-next] net: Cleanup skb cloning by adding SKB_FCLONE_FREE
From: Vijay Subramanian @ 2014-10-02 17:00 UTC (permalink / raw)
To: netdev; +Cc: davem, edumazet, Vijay Subramanian
SKB_FCLONE_UNAVAILABLE has overloaded meaning depending on type of skb.
1: If skb is allocated from head_cache, it indicates fclone is not available.
2: If skb is a companion fclone skb (allocated from fclone_cache), it indicates
it is available to be used.
To avoid confusion for case 2 above, this patch replaces
SKB_FCLONE_UNAVAILABLE with SKB_FCLONE_FREE where appropriate. For fclone
companion skbs, this indicates it is free for use.
SKB_FCLONE_UNAVAILABLE will now simply indicate skb is from head_cache and
cannot / will not have a companion fclone.
Signed-off-by: Vijay Subramanian <subramanian.vijay@gmail.com>
---
V1-->V2: Comment all states
include/linux/skbuff.h | 7 ++++---
net/core/skbuff.c | 8 ++++----
2 files changed, 8 insertions(+), 7 deletions(-)
diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index 7c5036d..3a5ec76 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -339,9 +339,10 @@ struct skb_shared_info {
enum {
- SKB_FCLONE_UNAVAILABLE,
- SKB_FCLONE_ORIG,
- SKB_FCLONE_CLONE,
+ SKB_FCLONE_UNAVAILABLE, /* skb has no fclone (from head_cache) */
+ SKB_FCLONE_ORIG, /* orig skb (from fclone_cache) */
+ SKB_FCLONE_CLONE, /* companion fclone skb (from fclone_cache) */
+ SKB_FCLONE_FREE, /* this companion fclone skb is available */
};
enum {
diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index f77e648..6f4e359 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -265,7 +265,7 @@ struct sk_buff *__alloc_skb(unsigned int size, gfp_t gfp_mask,
skb->fclone = SKB_FCLONE_ORIG;
atomic_set(&fclones->fclone_ref, 1);
- fclones->skb2.fclone = SKB_FCLONE_UNAVAILABLE;
+ fclones->skb2.fclone = SKB_FCLONE_FREE;
fclones->skb2.pfmemalloc = pfmemalloc;
}
out:
@@ -542,7 +542,7 @@ static void kfree_skbmem(struct sk_buff *skb)
fclones = container_of(skb, struct sk_buff_fclones, skb2);
/* Warning : We must perform the atomic_dec_and_test() before
- * setting skb->fclone back to SKB_FCLONE_UNAVAILABLE, otherwise
+ * setting skb->fclone back to SKB_FCLONE_FREE, otherwise
* skb_clone() could set clone_ref to 2 before our decrement.
* Anyway, if we are going to free the structure, no need to
* rewrite skb->fclone.
@@ -553,7 +553,7 @@ static void kfree_skbmem(struct sk_buff *skb)
/* The clone portion is available for
* fast-cloning again.
*/
- skb->fclone = SKB_FCLONE_UNAVAILABLE;
+ skb->fclone = SKB_FCLONE_FREE;
}
break;
}
@@ -874,7 +874,7 @@ struct sk_buff *skb_clone(struct sk_buff *skb, gfp_t gfp_mask)
return NULL;
if (skb->fclone == SKB_FCLONE_ORIG &&
- n->fclone == SKB_FCLONE_UNAVAILABLE) {
+ n->fclone == SKB_FCLONE_FREE) {
n->fclone = SKB_FCLONE_CLONE;
/* As our fastclone was free, clone_ref must be 1 at this point.
* We could use atomic_inc() here, but it is faster
--
1.9.1
^ permalink raw reply related
* [PATCH 0/2 NEXT] Some late fixes for rtlwifi
From: Larry Finger @ 2014-10-02 17:00 UTC (permalink / raw)
To: linville; +Cc: linux-wireless, troy_tan, Larry Finger, netdev
These patches fix a Kconfig error for RTL8192EE, and some static checker warnings
reported by Dan Carpenter.
Signed-off-by: Larry Finger <Larry.Finger@lwfinger.net>
---
Larry Finger (2):
rtlwifi: Fix Kconfig for RTL8192EE
rtlwifi: Fix static checker warnings for various drivers
drivers/net/wireless/rtlwifi/Kconfig | 3 ++-
drivers/net/wireless/rtlwifi/rtl8188ee/trx.c | 7 -------
drivers/net/wireless/rtlwifi/rtl8192ce/trx.c | 4 ----
drivers/net/wireless/rtlwifi/rtl8192ee/hw.c | 24 ++++++++++++------------
drivers/net/wireless/rtlwifi/rtl8192ee/trx.c | 7 -------
drivers/net/wireless/rtlwifi/rtl8192se/trx.c | 4 ----
drivers/net/wireless/rtlwifi/rtl8723ae/trx.c | 6 ------
drivers/net/wireless/rtlwifi/rtl8723be/trx.c | 7 -------
drivers/net/wireless/rtlwifi/rtl8821ae/trx.c | 7 -------
9 files changed, 14 insertions(+), 55 deletions(-)
--
1.8.4.5
^ permalink raw reply
* [PATCH 1/2 NEXT] rtlwifi: Fix Kconfig for RTL8192EE
From: Larry Finger @ 2014-10-02 17:00 UTC (permalink / raw)
To: linville; +Cc: linux-wireless, troy_tan, Larry Finger, netdev
In-Reply-To: <1412269254-19983-1-git-send-email-Larry.Finger@lwfinger.net>
The driver needs btcoexist, but Kconfig fails to select it. This omission
could cause build errors for some configurations.
Signed-off-by: Larry Finger <Larry.Finger@lwfinger.net>
---
drivers/net/wireless/rtlwifi/Kconfig | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/net/wireless/rtlwifi/Kconfig b/drivers/net/wireless/rtlwifi/Kconfig
index 552742e..5cf509d 100644
--- a/drivers/net/wireless/rtlwifi/Kconfig
+++ b/drivers/net/wireless/rtlwifi/Kconfig
@@ -86,6 +86,7 @@ config RTL8192EE
depends on PCI
select RTLWIFI
select RTLWIFI_PCI
+ select RTLBTCOEXIST
---help---
This is the driver for Realtek RTL8192EE 802.11n PCIe
wireless network adapters.
@@ -147,7 +148,7 @@ config RTL8723_COMMON
config RTLBTCOEXIST
tristate
- depends on RTL8723AE || RTL8723BE || RTL8821AE
+ depends on RTL8723AE || RTL8723BE || RTL8821AE || RTL8192EE
default y
endif
--
1.8.4.5
^ permalink raw reply related
* [PATCH 2/2 NEXT] rtlwifi: Fix static checker warnings for various drivers
From: Larry Finger @ 2014-10-02 17:00 UTC (permalink / raw)
To: linville; +Cc: linux-wireless, troy_tan, Larry Finger, netdev, Dan Carpenter
In-Reply-To: <1412269254-19983-1-git-send-email-Larry.Finger@lwfinger.net>
Indenting errors yielded the following static checker warnings:
drivers/net/wireless/rtlwifi/rtl8192ee/hw.c:533 rtl92ee_set_hw_reg() warn: add curly braces? (if)
drivers/net/wireless/rtlwifi/rtl8192ee/hw.c:539 rtl92ee_set_hw_reg() warn: add curly braces? (if)
An unreleased version of the static checker also reported:
drivers/net/wireless/rtlwifi/rtl8723be/trx.c:550 rtl8723be_rx_query_desc() warn: 'hdr' can't be NULL.
drivers/net/wireless/rtlwifi/rtl8188ee/trx.c:621 rtl88ee_rx_query_desc() warn: 'hdr' can't be NULL.
drivers/net/wireless/rtlwifi/rtl8192ee/trx.c:567 rtl92ee_rx_query_desc() warn: 'hdr' can't be NULL.
drivers/net/wireless/rtlwifi/rtl8821ae/trx.c:758 rtl8821ae_rx_query_desc() warn: 'hdr' can't be NULL.
drivers/net/wireless/rtlwifi/rtl8723ae/trx.c:494 rtl8723e_rx_query_desc() warn: 'hdr' can't be NULL.
drivers/net/wireless/rtlwifi/rtl8192se/trx.c:315 rtl92se_rx_query_desc() warn: 'hdr' can't be NULL.
drivers/net/wireless/rtlwifi/rtl8192ce/trx.c:392 rtl92ce_rx_query_desc() warn: 'hdr' can't be NULL.
All of these are fixed.
Signed-off-by: Larry Finger <Larry.Finger@lwfinger.net>
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
Cc: Dan Carpenter <dan.carpenter@oracle.com>
---
drivers/net/wireless/rtlwifi/rtl8188ee/trx.c | 7 -------
drivers/net/wireless/rtlwifi/rtl8192ce/trx.c | 4 ----
drivers/net/wireless/rtlwifi/rtl8192ee/hw.c | 24 ++++++++++++------------
drivers/net/wireless/rtlwifi/rtl8192ee/trx.c | 7 -------
drivers/net/wireless/rtlwifi/rtl8192se/trx.c | 4 ----
drivers/net/wireless/rtlwifi/rtl8723ae/trx.c | 6 ------
drivers/net/wireless/rtlwifi/rtl8723be/trx.c | 7 -------
drivers/net/wireless/rtlwifi/rtl8821ae/trx.c | 7 -------
8 files changed, 12 insertions(+), 54 deletions(-)
diff --git a/drivers/net/wireless/rtlwifi/rtl8188ee/trx.c b/drivers/net/wireless/rtlwifi/rtl8188ee/trx.c
index cf56ec8..df549c9 100644
--- a/drivers/net/wireless/rtlwifi/rtl8188ee/trx.c
+++ b/drivers/net/wireless/rtlwifi/rtl8188ee/trx.c
@@ -618,13 +618,6 @@ bool rtl88ee_rx_query_desc(struct ieee80211_hw *hw,
* to decrypt it
*/
if (status->decrypted) {
- if (!hdr) {
- WARN_ON_ONCE(true);
- pr_err("decrypted is true but hdr NULL, from skb %p\n",
- rtl_get_hdr(skb));
- return false;
- }
-
if ((!_ieee80211_is_robust_mgmt_frame(hdr)) &&
(ieee80211_has_protected(hdr->frame_control)))
rx_status->flag |= RX_FLAG_DECRYPTED;
diff --git a/drivers/net/wireless/rtlwifi/rtl8192ce/trx.c b/drivers/net/wireless/rtlwifi/rtl8192ce/trx.c
index c140123..2fb9c7a 100644
--- a/drivers/net/wireless/rtlwifi/rtl8192ce/trx.c
+++ b/drivers/net/wireless/rtlwifi/rtl8192ce/trx.c
@@ -389,10 +389,6 @@ bool rtl92ce_rx_query_desc(struct ieee80211_hw *hw,
* to decrypt it
*/
if (stats->decrypted) {
- if (!hdr) {
- /* In testing, hdr was NULL here */
- return false;
- }
if ((_ieee80211_is_robust_mgmt_frame(hdr)) &&
(ieee80211_has_protected(hdr->frame_control)))
rx_status->flag &= ~RX_FLAG_DECRYPTED;
diff --git a/drivers/net/wireless/rtlwifi/rtl8192ee/hw.c b/drivers/net/wireless/rtlwifi/rtl8192ee/hw.c
index 85d0d58..dfdc9b2 100644
--- a/drivers/net/wireless/rtlwifi/rtl8192ee/hw.c
+++ b/drivers/net/wireless/rtlwifi/rtl8192ee/hw.c
@@ -530,18 +530,18 @@ void rtl92ee_set_hw_reg(struct ieee80211_hw *hw, u8 variable, u8 *val)
fac = (1 << (fac + 2));
if (fac > 0xf)
fac = 0xf;
- for (i = 0; i < 4; i++) {
- if ((reg[i] & 0xf0) > (fac << 4))
- reg[i] = (reg[i] & 0x0f) |
- (fac << 4);
- if ((reg[i] & 0x0f) > fac)
- reg[i] = (reg[i] & 0xf0) | fac;
- rtl_write_byte(rtlpriv,
- (REG_AGGLEN_LMT + i),
- reg[i]);
- }
- RT_TRACE(rtlpriv, COMP_MLME, DBG_LOUD,
- "Set HW_VAR_AMPDU_FACTOR:%#x\n", fac);
+ for (i = 0; i < 4; i++) {
+ if ((reg[i] & 0xf0) > (fac << 4))
+ reg[i] = (reg[i] & 0x0f) |
+ (fac << 4);
+ if ((reg[i] & 0x0f) > fac)
+ reg[i] = (reg[i] & 0xf0) | fac;
+ rtl_write_byte(rtlpriv,
+ (REG_AGGLEN_LMT + i),
+ reg[i]);
+ }
+ RT_TRACE(rtlpriv, COMP_MLME, DBG_LOUD,
+ "Set HW_VAR_AMPDU_FACTOR:%#x\n", fac);
}
}
break;
diff --git a/drivers/net/wireless/rtlwifi/rtl8192ee/trx.c b/drivers/net/wireless/rtlwifi/rtl8192ee/trx.c
index 83edd95..2fcbef1 100644
--- a/drivers/net/wireless/rtlwifi/rtl8192ee/trx.c
+++ b/drivers/net/wireless/rtlwifi/rtl8192ee/trx.c
@@ -564,13 +564,6 @@ bool rtl92ee_rx_query_desc(struct ieee80211_hw *hw,
* to decrypt it
*/
if (status->decrypted) {
- if (!hdr) {
- WARN_ON_ONCE(true);
- pr_err("decrypted is true but hdr NULL, from skb %p\n",
- rtl_get_hdr(skb));
- return false;
- }
-
if ((!_ieee80211_is_robust_mgmt_frame(hdr)) &&
(ieee80211_has_protected(hdr->frame_control)))
rx_status->flag |= RX_FLAG_DECRYPTED;
diff --git a/drivers/net/wireless/rtlwifi/rtl8192se/trx.c b/drivers/net/wireless/rtlwifi/rtl8192se/trx.c
index 2b3c78b..b358ebc 100644
--- a/drivers/net/wireless/rtlwifi/rtl8192se/trx.c
+++ b/drivers/net/wireless/rtlwifi/rtl8192se/trx.c
@@ -312,10 +312,6 @@ bool rtl92se_rx_query_desc(struct ieee80211_hw *hw, struct rtl_stats *stats,
hdr = (struct ieee80211_hdr *)(skb->data +
stats->rx_drvinfo_size + stats->rx_bufshift);
- if (!hdr) {
- /* during testing, hdr was NULL here */
- return false;
- }
if ((_ieee80211_is_robust_mgmt_frame(hdr)) &&
(ieee80211_has_protected(hdr->frame_control)))
rx_status->flag &= ~RX_FLAG_DECRYPTED;
diff --git a/drivers/net/wireless/rtlwifi/rtl8723ae/trx.c b/drivers/net/wireless/rtlwifi/rtl8723ae/trx.c
index 1da2367..d372cca 100644
--- a/drivers/net/wireless/rtlwifi/rtl8723ae/trx.c
+++ b/drivers/net/wireless/rtlwifi/rtl8723ae/trx.c
@@ -491,12 +491,6 @@ bool rtl8723e_rx_query_desc(struct ieee80211_hw *hw,
* to decrypt it
*/
if (status->decrypted) {
- if (!hdr) {
- WARN_ON_ONCE(true);
- pr_err("decrypted is true but hdr NULL, from skb %p\n",
- rtl_get_hdr(skb));
- return false;
- }
if ((!_ieee80211_is_robust_mgmt_frame(hdr)) &&
(ieee80211_has_protected(hdr->frame_control)))
rx_status->flag |= RX_FLAG_DECRYPTED;
diff --git a/drivers/net/wireless/rtlwifi/rtl8723be/trx.c b/drivers/net/wireless/rtlwifi/rtl8723be/trx.c
index 9679cd2..d6a1c70 100644
--- a/drivers/net/wireless/rtlwifi/rtl8723be/trx.c
+++ b/drivers/net/wireless/rtlwifi/rtl8723be/trx.c
@@ -547,13 +547,6 @@ bool rtl8723be_rx_query_desc(struct ieee80211_hw *hw,
* to decrypt it
*/
if (status->decrypted) {
- if (!hdr) {
- WARN_ON_ONCE(true);
- pr_err("decrypted is true but hdr NULL, from skb %p\n",
- rtl_get_hdr(skb));
- return false;
- }
-
if ((!_ieee80211_is_robust_mgmt_frame(hdr)) &&
(ieee80211_has_protected(hdr->frame_control)))
rx_status->flag |= RX_FLAG_DECRYPTED;
diff --git a/drivers/net/wireless/rtlwifi/rtl8821ae/trx.c b/drivers/net/wireless/rtlwifi/rtl8821ae/trx.c
index 7ece0ef..383b86b 100644
--- a/drivers/net/wireless/rtlwifi/rtl8821ae/trx.c
+++ b/drivers/net/wireless/rtlwifi/rtl8821ae/trx.c
@@ -755,13 +755,6 @@ bool rtl8821ae_rx_query_desc(struct ieee80211_hw *hw,
* to decrypt it
*/
if (status->decrypted) {
- if (!hdr) {
- WARN_ON_ONCE(true);
- pr_err("decrypted is true but hdr NULL, from skb %p\n",
- rtl_get_hdr(skb));
- return false;
- }
-
if ((!_ieee80211_is_robust_mgmt_frame(hdr)) &&
(ieee80211_has_protected(hdr->frame_control)))
rx_status->flag |= RX_FLAG_DECRYPTED;
--
1.8.4.5
^ permalink raw reply related
* phpBB 3.1.0 new version
From: phpbbaid @ 2014-10-02 17:05 UTC (permalink / raw)
To: netdev
phpBB 3.1.0 new version is out .
Please update your forum to the latest version .
We provide paid support if you are interested, please, reply to this email
Thank you
^ permalink raw reply
* Re: [PATCH net-next v6 2/2] bonding: Simplify the xmit function for modes that use xmit_hash
From: Mahesh Bandewar @ 2014-10-02 17:19 UTC (permalink / raw)
To: Cong Wang
Cc: Jay Vosburgh, Veaceslav Falico, Andy Gospodarek, David Miller,
netdev, Eric Dumazet, Maciej Zenczykowski
In-Reply-To: <CAHA+R7PiP2Ce-+3S6Uy5kkKjk1sN6uE2gXzvL_HtfnyVHLoKoQ@mail.gmail.com>
On Thu, Oct 2, 2014 at 10:10 AM, Cong Wang <cwang@twopensource.com> wrote:
> On Wed, Oct 1, 2014 at 1:38 AM, Mahesh Bandewar <maheshb@google.com> wrote:
>> +int bond_update_slave_arr(struct bonding *bond, struct slave *skipslave)
>> +{
>> + struct slave *slave;
>> + struct list_head *iter;
>> + struct bond_up_slave *new_arr, *old_arr;
>> + int slaves_in_agg;
>> + int agg_id = 0;
>> + int ret = 0;
>> +
>> +#ifdef CONFIG_LOCKDEP
>> + WARN_ON(lockdep_is_held(&bond->mode_lock));
>> +#endif
>
>
> I think you can use lockdep_is_held().
>
>> +
>> + new_arr = kzalloc(offsetof(struct bond_up_slave, arr[bond->slave_cnt]),
>> + GFP_KERNEL);
>> + if (!new_arr) {
>> + ret = -ENOMEM;
>> + pr_err("Failed to build slave-array.\n");
>> + goto out;
>> + }
>
>
> No need to print an error message for OOM, it is already noisy. :)
>
Agreed OOM condition is noisy but failing silently would not help
debugging hence adding the message.
>
>> + if (BOND_MODE(bond) == BOND_MODE_8023AD) {
>> + struct ad_info ad_info;
>> +
>> + if (bond_3ad_get_active_agg_info(bond, &ad_info)) {
>> + pr_debug("bond_3ad_get_active_agg_info failed\n");
>
>
> I suspect how useful this debug info is since your patch is almost ready
> to merge.
>
It could be useful for someone else too :)
>> + kfree_rcu(new_arr, rcu);
>> + /* No active aggragator means its not safe to use
>
> s/its/it's/
>
thanks
>> + * the previous array.
>> + */
>> + old_arr = rtnl_dereference(bond->slave_arr);
>> + if (old_arr) {
>> + RCU_INIT_POINTER(bond->slave_arr, NULL);
>> + kfree_rcu(old_arr, rcu);
>> + }
>> + goto out;
>> + }
>> + slaves_in_agg = ad_info.ports;
>> + agg_id = ad_info.aggregator_id;
>> + }
>
>
>
> Thanks.
^ permalink raw reply
* Re: [RFC PATCH linux 2/2] fs/proc: use a hash table for the directory entries
From: Alexey Dobriyan @ 2014-10-02 17:28 UTC (permalink / raw)
To: Nicolas Dichtel
Cc: netdev, linux-kernel, davem, ebiederm, akpm, rui.xiang, viro,
oleg, gorcunov, kirill.shutemov, grant.likely, tytso,
Thierry Herbelot
In-Reply-To: <1412263501-6572-3-git-send-email-nicolas.dichtel@6wind.com>
On Thu, Oct 02, 2014 at 05:25:01PM +0200, Nicolas Dichtel wrote:
> +static inline unsigned int proc_pde_name_hash(const unsigned char *name,
> + const unsigned int len)
> +{
> + return full_name_hash(name, len) & PROC_PDE_HASH_MASK;
> +}
PDE already stands for "proc dir entry" :^)
Alexey
^ permalink raw reply
* Re: [PATCH net-next v6 2/2] bonding: Simplify the xmit function for modes that use xmit_hash
From: Mahesh Bandewar @ 2014-10-02 17:28 UTC (permalink / raw)
To: David Laight
Cc: Jay Vosburgh, Veaceslav Falico, Andy Gospodarek, David Miller,
netdev, Eric Dumazet, Maciej Zenczykowski
In-Reply-To: <063D6719AE5E284EB5DD2968C1650D6D174A2CF5@AcuExch.aculab.com>
On Thu, Oct 2, 2014 at 2:42 PM, David Laight <David.Laight@aculab.com> wrote:
> From: Mahesh Bandewar
>> On Wed, Oct 1, 2014 at 9:49 PM, Jay Vosburgh <jay.vosburgh@canonical.com> wrote:
>> > Mahesh Bandewar <maheshb@google.com> wrote:
> ...
>> >> * Select aggregation groups, and assign each port for it's aggregetor. The
>> >> * selection logic is called in the inititalization (after all the handshkes),
>> >> * and after every lacpdu receive (if selected is off).
>> >> */
>> >>-static void ad_port_selection_logic(struct port *port)
>> >>+static void ad_port_selection_logic(struct port *port, bool *update_slave_arr)
>> >
>> > Since this function is void, why not have it return a value
>> > instead of the bool *update_slave_arr? That would eliminate the need
>> > for some call sites to pass a "dummy" to the function. This comment
>> > applies to ad_agg_selection_logic and ad_enable_collecting_distributing
>> > as well.
>> >
>> Yes, I had similar discussion with Nik earlier and overloading the
>> return value did not feel clean and future-proof and hence decided to
>> take this approach.
>
> What overload?
> Returning values by reference parameters isn't really a good idea.
> It kills performance and optimisations.
> If you ever need a second return value then solve the problem then.
>
Please show me how much performance we are loosing by taking this
approach... otherwise this argument is bogus!
> David
>
>
>
^ permalink raw reply
* Re: [net-next PATCH V6 0/2] qdisc: bulk dequeue support
From: Eric Dumazet @ 2014-10-02 17:32 UTC (permalink / raw)
To: Florian Westphal
Cc: Tom Herbert, Jesper Dangaard Brouer, Linux Netdev List,
David S. Miller, Hannes Frederic Sowa, Daniel Borkmann,
Jamal Hadi Salim, Alexander Duyck, John Fastabend, Dave Taht,
Toke Høiland-Jørgensen
In-Reply-To: <20141002165215.GJ1803@breakpoint.cc>
On Thu, 2014-10-02 at 18:52 +0200, Florian Westphal wrote:
> Tom Herbert <therbert@google.com> wrote:
> > On Wed, Oct 1, 2014 at 1:35 PM, Jesper Dangaard Brouer
> > <brouer@redhat.com> wrote:
> > > This patchset uses DaveM's recent API changes to dev_hard_start_xmit(),
> > > from the qdisc layer, to implement dequeue bulking.
> > >
> > > Patch01: "qdisc: bulk dequeue support for qdiscs with TCQ_F_ONETXQUEUE"
> > > - Implement basic qdisc dequeue bulking
> > > - This time, 100% relying on BQL limits, no magic safe-guard constants
> > >
> > > Patch02: "qdisc: dequeue bulking also pickup GSO/TSO packets"
> > > - Extend bulking to bulk several GSO/TSO packets
> > > - Seperate patch, as it introduce a small regression, see test section.
> > >
> > > We do have a patch03, which exports a userspace tunable as a BQL
> > > tunable, that can byte-cap or disable the bulking/bursting. But we
> > > could not agree on it internally, thus not sending it now. We
> > > basically strive to avoid adding any new userspace tunable.
> > >
> > Unfortunately we probably still need something. If BQL were disabled
> > (by setting BQL min_limit to infinity) then we'll always dequeue all
> > the packets in the qdisc. Disabling BQL might be legitimate in
> > deployment if say a bug is found in a device that prevents prompt
> > transmit completions for some corner case.
>
> Hmm. Thats confusing.
>
> So you are saying to disable bql one should do
>
> cat limit_max > limit_min ?
>
> But thats not the same as having a bql-unaware driver.
>
> Seems to get same behavior as non-bql aware driver (where
> dql_avail always returns 0 since num_queued and adj_limit remain at 0)
> is to set
>
> echo 0 > limit_max
>
> ... which makes dql_avail() return 0, which then also turns off bulk
> dequeue.
So Tom point is that we might have a BQL enabled driver, but for some
reason admin wants to set limit_min to 10000000.
Then the admin wants also to restrict the xmit_more batches to 13
packets.
Then a debugging facility might be nice to have.
What about adding 2 attributes :
/sys/class/net/ethX/queues/tx-Y/xmit_more_inflight (live number of
deferred frames on this TX queue, waiting a doorbell to kick the NIC)
/sys/class/net/ethX/queues/tx-Y/xmit_more_limit (default to 8)
^ permalink raw reply
* Re: [net-next PATCH V6 0/2] qdisc: bulk dequeue support
From: Tom Herbert @ 2014-10-02 17:35 UTC (permalink / raw)
To: Florian Westphal
Cc: Jesper Dangaard Brouer, Linux Netdev List, David S. Miller,
Eric Dumazet, Hannes Frederic Sowa, Daniel Borkmann,
Jamal Hadi Salim, Alexander Duyck, John Fastabend, Dave Taht,
Toke Høiland-Jørgensen
In-Reply-To: <20141002165215.GJ1803@breakpoint.cc>
On Thu, Oct 2, 2014 at 9:52 AM, Florian Westphal <fw@strlen.de> wrote:
> Tom Herbert <therbert@google.com> wrote:
>> On Wed, Oct 1, 2014 at 1:35 PM, Jesper Dangaard Brouer
>> <brouer@redhat.com> wrote:
>> > This patchset uses DaveM's recent API changes to dev_hard_start_xmit(),
>> > from the qdisc layer, to implement dequeue bulking.
>> >
>> > Patch01: "qdisc: bulk dequeue support for qdiscs with TCQ_F_ONETXQUEUE"
>> > - Implement basic qdisc dequeue bulking
>> > - This time, 100% relying on BQL limits, no magic safe-guard constants
>> >
>> > Patch02: "qdisc: dequeue bulking also pickup GSO/TSO packets"
>> > - Extend bulking to bulk several GSO/TSO packets
>> > - Seperate patch, as it introduce a small regression, see test section.
>> >
>> > We do have a patch03, which exports a userspace tunable as a BQL
>> > tunable, that can byte-cap or disable the bulking/bursting. But we
>> > could not agree on it internally, thus not sending it now. We
>> > basically strive to avoid adding any new userspace tunable.
>> >
>> Unfortunately we probably still need something. If BQL were disabled
>> (by setting BQL min_limit to infinity) then we'll always dequeue all
>> the packets in the qdisc. Disabling BQL might be legitimate in
>> deployment if say a bug is found in a device that prevents prompt
>> transmit completions for some corner case.
>
> Hmm. Thats confusing.
>
> So you are saying to disable bql one should do
>
> cat limit_max > limit_min ?
>
echo max > limit_min
echo max > limit_max
"Disables" BQL by forcing the limit to be really big.
> But thats not the same as having a bql-unaware driver.
>
Yes, that's true. This does not disable the accounting and limit check
which is where a bug would manifest itself.
> Seems to get same behavior as non-bql aware driver (where
> dql_avail always returns 0 since num_queued and adj_limit remain at 0)
> is to set
>
That doesn't work for BQL, if dql_avail returning zero means we can
queue only one packet.
> echo 0 > limit_max
>
> ... which makes dql_avail() return 0, which then also turns off bulk
> dequeue.
>
> Confused,
> Florian
I withdraw my comment.
^ permalink raw reply
* <Reply ASAP>
From: Mr. James @ 2014-10-02 8:54 UTC (permalink / raw)
To: Recipients
Sequel to your non-response to my previous email, I am re-sending this to you again thus; A deceased client of mine who died of a heart-related ailment about 3 years ago left behind some funds which I want you to assist in retriving and distributing. Reply so I can give you details on what is needed to be done.
Regards,
James.
^ permalink raw reply
* Re: [RFC PATCH linux 2/2] fs/proc: use a hash table for the directory entries
From: Eric W. Biederman @ 2014-10-02 18:01 UTC (permalink / raw)
To: Nicolas Dichtel
Cc: netdev, linux-kernel, davem, akpm, adobriyan, rui.xiang, viro,
oleg, gorcunov, kirill.shutemov, grant.likely, tytso,
Thierry Herbelot
In-Reply-To: <1412263501-6572-3-git-send-email-nicolas.dichtel@6wind.com>
Nicolas Dichtel <nicolas.dichtel@6wind.com> writes:
> From: Thierry Herbelot <thierry.herbelot@6wind.com>
>
> The current implementation for the directories in /proc is using a single
> linked list. This is slow when handling directories with large numbers of
> entries (eg netdevice-related entries when lots of tunnels are opened).
>
> This patch enables multiple linked lists. A hash based on the entry name is
> used to select the linked list for one given entry.
>
> The speed creation of netdevices is faster as shorter linked lists must be
> scanned when adding a new netdevice.
Is the directory of primary concern /proc/net/dev/snmp6 ?
Unless I have configured my networking stack weird by mistake that
is the only directory under /proc/net that grows when we add an
interface.
I just want to make certain I am seeing the same things that you are
seeing.
I feel silly for overlooking this directory when the rest of the
scalability work was done.
> Here are some numbers:
>
> dummy30000.batch contains 30 000 times 'link add type dummy'.
>
> Before the patch:
> time ip -b dummy30000.batch
> real 2m32.221s
> user 0m0.380s
> sys 2m30.610s
>
> After the patch:
> time ip -b dummy30000.batch
> real 1m57.190s
> user 0m0.350s
> sys 1m56.120s
>
> The single 'subdir' list head is replaced by a subdir hash table. The subdir
> hash buckets are only allocated for directories. The number of hash buckets
> is a compile-time parameter.
That looks like a nice speed up. A couple of things.
With sysfs and sysctl when faced this class of challenge we used an
rbtree instead of a hash table. That should use less memory and scale
better.
I am concerned about a fixed sized hash table moving the location where
we fall off a cliff but not removing the cliff itself.
I suppose it would be possible to use the new fancy resizable hash
tables but previous work on sysctl and sysfs suggests that we don't look
up these entries sufficiently to require a hash table. We just need a
data structure that doesn't fall over at scale, and the rbtrees seem to
do that very nicely.
> For all functions which handle directory entries, an additional check on the
> directory nature of the dir entry ensures that pde_hash_buckets was allocated.
> This check was not needed as subdir was present for all dir entries, whether
> actual directories or simple files.
That bit of logic seems reasonable.
Eric
^ permalink raw reply
* Re: [Patch net-next] net_sched: avoid calling tcf_unbind_filter() in call_rcu callback
From: John Fastabend @ 2014-10-02 18:04 UTC (permalink / raw)
To: David Miller; +Cc: xiyou.wangcong, netdev, john.r.fastabend
In-Reply-To: <20141001.220123.1054083748022563765.davem@davemloft.net>
On 10/01/2014 07:01 PM, David Miller wrote:
> From: Cong Wang <xiyou.wangcong@gmail.com>
> Date: Tue, 30 Sep 2014 16:07:24 -0700
>
>> This fixes the following crash:
> ...
>> tp could be freed in call_rcu callback too, the order is not guaranteed.
>>
>> Cc: John Fastabend <john.r.fastabend@intel.com>
>> Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>
>
> Applied, and I added John's description of why this is legal to the
> commit message.
> --
Thanks, I'll have another series shortly to fix the
other classifiers with the same issue and pull 'tp'
out of the ematch stuff.
Passing around free'd pointers that never get used
through the ematch code wont cause a crash but there is
no reason to propagate it like this.
.John
--
John Fastabend Intel Corporation
^ permalink raw reply
* Re: linux-next: manual merge of the net-next tree with the net tree
From: David Miller @ 2014-10-02 18:27 UTC (permalink / raw)
To: sfr; +Cc: netdev, linux-next, linux-kernel, hayeswang
In-Reply-To: <20141002141641.0e43a18e@canb.auug.org.au>
From: Stephen Rothwell <sfr@canb.auug.org.au>
Date: Thu, 2 Oct 2014 14:16:41 +1000
> Today's linux-next merge of the net-next tree got a conflict in
> drivers/net/usb/r8152.c between commit 204c87041289 ("r8152: remove
> clearing bp") from the net tree and commit 8ddfa07778af ("r8152: use
> usleep_range") from the net-next tree.
>
> I fixed it up (the former removed some of the code updated by the
> latter) and can carry the fix as necessary (no action is required).
I'm merging net into net-next today and thus resolving this.
Thanks Stephen!
^ permalink raw reply
* bug: race in team_{notify_peers,mcast_rejoin} scheduling
From: Joe Lawrence @ 2014-10-02 18:50 UTC (permalink / raw)
To: netdev, Jiri Pirko
Hello Jiri,
Occasionally on boot I noticed that team_notify_peers_work would get
*very* busy.
With the following debugging added to team_notify_peers:
netdev_info(team->dev, "%s(%p)\n", __func__, team);
dump_stack();
I saw the following:
% dmesg | grep -e 'team[0-9]: team_notify_peers' -e 'port_enable' -e 'port_disable'
[ 68.340861] team0: team_notify_peers(ffff88104ffa4de0)
[ 68.743264] [<ffffffffa034fd38>] team_port_enable.part.40+0x78/0x90 [team]
[ 69.622395] team0: team_notify_peers(ffff88104ffa4de0)
[ 69.966758] [<ffffffffa034ef63>] team_port_disable+0x123/0x160 [team]
[ 71.099263] team0: team_notify_peers(ffff88104ffa4de0)
[ 71.466243] [<ffffffffa034fd38>] team_port_enable.part.40+0x78/0x90 [team]
[ 72.383788] team0: team_notify_peers(ffff88104ffa4de0)
[ 72.744778] [<ffffffffa034ef63>] team_port_disable+0x123/0x160 [team]
[ 73.476190] team0: team_notify_peers(ffff88104ffa4de0)
[ 73.830592] [<ffffffffa034fd38>] team_port_enable.part.40+0x78/0x90 [team]
[ 74.796738] team1: team_notify_peers(ffff88104f5df080)
[ 75.165577] [<ffffffffa034fd38>] team_port_enable.part.40+0x78/0x90 [team]
[ 75.694968] team1: team_notify_peers(ffff88104f5df080)
[ 75.694984] [<ffffffffa034ef63>] team_port_disable+0x123/0x160 [team]
[ 77.316488] team1: team_notify_peers(ffff88104f5df080)
[ 77.663122] [<ffffffffa034fd38>] team_port_enable.part.40+0x78/0x90 [team]
[ 78.470488] team1: team_notify_peers(ffff88104f5df080)
[ 78.814722] [<ffffffffa034ef63>] team_port_disable+0x123/0x160 [team]
[ 82.690765] team2: team_notify_peers(ffff88083d24df40)
[ 83.083540] [<ffffffffa034fd38>] team_port_enable.part.40+0x78/0x90 [team]
[ 83.942458] team2: team_notify_peers(ffff88083d24df40)
[ 84.286446] [<ffffffffa034ef63>] team_port_disable+0x123/0x160 [team]
[ 86.089955] team3: team_notify_peers(ffff88083fd14de0)
[ 86.453495] [<ffffffffa034fd38>] team_port_enable.part.40+0x78/0x90 [team]
[ 87.267773] team3: team_notify_peers(ffff88083fd14de0)
[ 87.610203] [<ffffffffa034ef63>] team_port_disable+0x123/0x160 [team]
which shows team_port_enable/disable getting invoked in short
succession. When looking at one of the team's
notify_peers.count_pending value, I saw that it was negative and slowly
counting down from 0xffff...ffff!
This lead me believe that there is a race condition present in
the .count_pending pattern that both team_notify_peers and
team_mcast_rejoin employ.
Can you comment on the following patch/workaround?
Thanks,
-- Joe
-->8-- -->8-- -->8--
>From b11d7dcd051a2f141c1eec0a43c4a4ddf0361d10 Mon Sep 17 00:00:00 2001
From: Joe Lawrence <joe.lawrence@stratus.com>
Date: Thu, 2 Oct 2014 14:24:26 -0400
Subject: [PATCH] team: avoid race condition in scheduling delayed work
When team_notify_peers and team_mcast_rejoin are called, they both reset
their respective .count_pending atomic variable. Then when the actual
worker function is executed, the variable is atomically decremented.
This pattern introduces a potential race condition where the
.count_pending rolls over and the worker function keeps rescheduling
until .count_pending decrements to zero again:
THREAD 1 THREAD 2
======== ========
team_notify_peers(teamX)
atomic_set count_pending = 1
schedule_delayed_work
team_notify_peers(teamX)
atomic_set count_pending = 1
team_notify_peers_work
atomic_dec_and_test
count_pending = 0
(return)
schedule_delayed_work
team_notify_peers_work
atomic_dec_and_test
count_pending = -1
schedule_delayed_work
(repeat until count_pending = 0)
Instead of assigning a new value to .count_pending, use atomic_add to
tack-on the additional desired worker function invocations.
Signed-off-by: Joe Lawrence <joe.lawrence@stratus.com>
---
drivers/net/team/team.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/net/team/team.c b/drivers/net/team/team.c
index d46df38..2b87e3f 100644
--- a/drivers/net/team/team.c
+++ b/drivers/net/team/team.c
@@ -647,7 +647,7 @@ static void team_notify_peers(struct team *team)
{
if (!team->notify_peers.count || !netif_running(team->dev))
return;
- atomic_set(&team->notify_peers.count_pending, team->notify_peers.count);
+ atomic_add(team->notify_peers.count, &team->notify_peers.count_pending);
schedule_delayed_work(&team->notify_peers.dw, 0);
}
@@ -687,7 +687,7 @@ static void team_mcast_rejoin(struct team *team)
{
if (!team->mcast_rejoin.count || !netif_running(team->dev))
return;
- atomic_set(&team->mcast_rejoin.count_pending, team->mcast_rejoin.count);
+ atomic_add(team->mcast_rejoin.count, &team->mcast_rejoin.count_pending);
schedule_delayed_work(&team->mcast_rejoin.dw, 0);
}
--
1.7.10.4
^ permalink raw reply related
* [PATCH net-next] Removed unused inet6 address state
From: Sébastien Barré @ 2014-10-02 19:15 UTC (permalink / raw)
To: David Miller
Cc: Sébastien Barré, netdev, Christoph Paasch, Herbert Xu
the inet6 state INET6_IFADDR_STATE_UP only appeared in its definition.
Cc: Christoph Paasch <christoph.paasch@uclouvain.be>
Cc: Herbert Xu <herbert@gondor.apana.org.au>
Signed-off-by: Sébastien Barré <sebastien.barre@uclouvain.be>
---
include/net/if_inet6.h | 1 -
1 file changed, 1 deletion(-)
diff --git a/include/net/if_inet6.h b/include/net/if_inet6.h
index d07b1a6..55a8d40 100644
--- a/include/net/if_inet6.h
+++ b/include/net/if_inet6.h
@@ -35,7 +35,6 @@ enum {
INET6_IFADDR_STATE_DAD,
INET6_IFADDR_STATE_POSTDAD,
INET6_IFADDR_STATE_ERRDAD,
- INET6_IFADDR_STATE_UP,
INET6_IFADDR_STATE_DEAD,
};
--
tg: (739e4a7..) net-next/remove_unused_inet6_state (depends on: net-next/master)
^ permalink raw reply related
* Re: [RFC PATCH net-next v2 0/5] netns: allow to identify peer netns
From: Eric W. Biederman @ 2014-10-02 19:20 UTC (permalink / raw)
To: nicolas.dichtel-pdR9zngts4EAvxtiuMwx3w
Cc: Network Development, Linux Containers,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
Andy Lutomirski, Stephen Hemminger, Cong Wang, Linux API,
Andrew Morton, David S. Miller
In-Reply-To: <542D5726.8070308-pdR9zngts4EAvxtiuMwx3w@public.gmane.org>
Nicolas Dichtel <nicolas.dichtel@6wind.com> writes:
> Le 29/09/2014 20:43, Eric W. Biederman a écrit :
>> Nicolas Dichtel <nicolas.dichtel@6wind.com> writes:
>>
>>> Le 26/09/2014 20:57, Eric W. Biederman a écrit :
>>>> Andy Lutomirski <luto@amacapital.net> writes:
>>>>
>>>>> On Fri, Sep 26, 2014 at 11:10 AM, Eric W. Biederman
>>>>> <ebiederm@xmission.com> wrote:
>>>>>> I see two ways to go with this.
>>>>>>
>>>>>> - A per network namespace table to that you can store ids for ``peer''
>>>>>> network namespaces. The table would need to be populated manually by
>>>>>> the likes of ip netns add.
>>>>>>
>>>>>> That flips the order of assignment and makes this idea solid.
>>> I have a preference for this solution, because it allows to have a full
>>> broadcast messages. When you have a lot of network interfaces (> 10k),
>>> it saves a lot of time to avoid another request to get all informations.
>>
>> My practical question is how often does it happen that we care?
> In fact, I don't think that scenarii with a lot of netns have a full mesh of
> x-netns interfaces. It will be more one "link" netns with the physical
> interface and all other with one interface with the link part in this "link"
> netns. Hence, only one nsid is needing in each netns.
I will buy that a full mesh is unlikely.
For people doing simulations anything physical has a limited number of
links.
For people wanting all to all connectivity setting up an internal
macvlan (or the equivalent) is likely much simpler and more efficient
that a full mesh.
So the question in my mind is how do we create these identifiers at need
(when we create the cross network namespace links) instead of at network
namespace creation time. I don't see an answer to that in your patches,
and perhaps it obvious.
Eric
_______________________________________________
Containers mailing list
Containers@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/containers
^ permalink raw reply
* Re: [RFC PATCH net-next v2 0/5] netns: allow to identify peer netns
From: Andy Lutomirski @ 2014-10-02 19:31 UTC (permalink / raw)
To: Eric W. Biederman
Cc: Andrew Morton, Network Development, Linux Containers,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
Stephen Hemminger, Cong Wang, Linux API, Nicolas Dichtel,
David S. Miller
In-Reply-To: <8761g2nurx.fsf-JOvCrm2gF+uungPnsOpG7nhyD016LWXt@public.gmane.org>
On Thu, Oct 2, 2014 at 12:20 PM, Eric W. Biederman
<ebiederm@xmission.com> wrote:
> Nicolas Dichtel <nicolas.dichtel@6wind.com> writes:
>
>> Le 29/09/2014 20:43, Eric W. Biederman a écrit :
>>> Nicolas Dichtel <nicolas.dichtel@6wind.com> writes:
>>>
>>>> Le 26/09/2014 20:57, Eric W. Biederman a écrit :
>>>>> Andy Lutomirski <luto@amacapital.net> writes:
>>>>>
>>>>>> On Fri, Sep 26, 2014 at 11:10 AM, Eric W. Biederman
>>>>>> <ebiederm@xmission.com> wrote:
>>>>>>> I see two ways to go with this.
>>>>>>>
>>>>>>> - A per network namespace table to that you can store ids for ``peer''
>>>>>>> network namespaces. The table would need to be populated manually by
>>>>>>> the likes of ip netns add.
>>>>>>>
>>>>>>> That flips the order of assignment and makes this idea solid.
>>>> I have a preference for this solution, because it allows to have a full
>>>> broadcast messages. When you have a lot of network interfaces (> 10k),
>>>> it saves a lot of time to avoid another request to get all informations.
>>>
>>> My practical question is how often does it happen that we care?
>> In fact, I don't think that scenarii with a lot of netns have a full mesh of
>> x-netns interfaces. It will be more one "link" netns with the physical
>> interface and all other with one interface with the link part in this "link"
>> netns. Hence, only one nsid is needing in each netns.
>
> I will buy that a full mesh is unlikely.
>
> For people doing simulations anything physical has a limited number of
> links.
>
> For people wanting all to all connectivity setting up an internal
> macvlan (or the equivalent) is likely much simpler and more efficient
> that a full mesh.
>
> So the question in my mind is how do we create these identifiers at need
> (when we create the cross network namespace links) instead of at network
> namespace creation time. I don't see an answer to that in your patches,
> and perhaps it obvious.
>
I wonder whether part of the problem is that we're thinking about
scoping wrong. What if we made the hierarchy more explicit?
For example, we could give each netns an admin-assigned identifier
(e.g. a 64-bit number, maybe required to be unique, maybe not)
relative to its containing userns. Then we could come up with a way
to identify user namespaces (i.e. inode number relative to containing
user ns, if that's well-defined).
From user code's perspective, netnses that are in the requester's
userns or its descendents are identified by a path through a (possibly
zero-length) sequence of userns ids followed by a netns id. Netnses
outside the requester's userns hierarchy cannot be named at all.
Would this make sense? It should keep the asymptotic complexity of
everything under control and, for users of very large numbers of
network namespaces with complex routing, it doesn't require a
correspondingly large number of fds. It would have the added benefit
of allowing the same scheme to be used for all the other namespace
types, although it could be a bit odd for pid namespaces, which really
do have their own hierarchy.
--Andy
_______________________________________________
Containers mailing list
Containers@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/containers
^ permalink raw reply
* Re: [RFC PATCH net-next v3 1/4] netns: add genl cmd to add and get peer netns ids
From: Eric W. Biederman @ 2014-10-02 19:33 UTC (permalink / raw)
To: Nicolas Dichtel
Cc: netdev-u79uwXL29TY76Z2rM5mHXA,
containers-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, luto-kltTT9wpgjJwATOyAt5JVQ,
stephen-OTpzqLSitTUnbdJkjeBofR2eb7JE58TQ,
cwang-xCSkyg8dI+0RB7SZvlqPiA, linux-api-u79uwXL29TY76Z2rM5mHXA,
akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b,
davem-fT/PcQaiUtIeIZ0/mPfg9Q
In-Reply-To: <1412257690-31253-2-git-send-email-nicolas.dichtel-pdR9zngts4EAvxtiuMwx3w@public.gmane.org>
Nicolas Dichtel <nicolas.dichtel-pdR9zngts4EAvxtiuMwx3w@public.gmane.org> writes:
> With this patch, a user can define an id for a peer netns by providing a FD or a
> PID. These ids are local to netns (ie valid only into one netns).
>
> This will be useful for netlink messages when a x-netns interface is
> dumped.
You have a "id -> struct net *" table but you don't have a
"struct net * -> id" table which looks like it will impact the
performance of peernet2id at scale.
Eric
^ 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