* [PATCH] net: mvneta: fix Tx interrupt delay
From: Willy Tarreau @ 2014-12-02 7:13 UTC (permalink / raw)
To: netdev; +Cc: Maggie Mae Roxas, Thomas Petazzoni, Gregory CLEMENT,
Ezequiel Garcia
The mvneta driver sets the amount of Tx coalesce packets to 16 by
default. Normally that does not cause any trouble since the driver
uses a much larger Tx ring size (532 packets). But some sockets
might run with very small buffers, much smaller than the equivalent
of 16 packets. This is what ping is doing for example, by setting
SNDBUF to 324 bytes rounded up to 2kB by the kernel.
The problem is that there is no documented method to force a specific
packet to emit an interrupt (eg: the last of the ring) nor is it
possible to make the NIC emit an interrupt after a given delay.
In this case, it causes trouble, because when ping sends packets over
its raw socket, the few first packets leave the system, and the first
15 packets will be emitted without an IRQ being generated, so without
the skbs being freed. And since the socket's buffer is small, there's
no way to reach that amount of packets, and the ping ends up with
"send: no buffer available" after sending 6 packets. Running with 3
instances of ping in parallel is enough to hide the problem, because
with 6 packets per instance, that's 18 packets total, which is enough
to grant a Tx interrupt before all are sent.
The original driver in the LSP kernel worked around this design flaw
by using a software timer to clean up the Tx descriptors. This timer
was slow and caused terrible network performance on some Tx-bound
workloads (such as routing) but was enough to make tools like ping
work correctly.
Instead here, we simply set the packet counts before interrupt to 1.
This ensures that each packet sent will produce an interrupt. NAPI
takes care of coalescing interrupts since the interrupt is disabled
once generated.
No measurable performance impact nor CPU usage were observed on small
nor large packets, including when saturating the link on Tx, and this
fixes tools like ping which rely on too small a send buffer. If one
wants to increase this value for certain workloads where it is safe
to do so, "ethtool -C $dev tx-frames" will override this default
setting.
This fix needs to be applied to stable kernels starting with 3.10.
Tested-By: Maggie Mae Roxas <maggie.mae.roxas@gmail.com>
Signed-off-by: Willy Tarreau <w@1wt.eu>
---
drivers/net/ethernet/marvell/mvneta.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/marvell/mvneta.c b/drivers/net/ethernet/marvell/mvneta.c
index 4762994..35bfba7 100644
--- a/drivers/net/ethernet/marvell/mvneta.c
+++ b/drivers/net/ethernet/marvell/mvneta.c
@@ -214,7 +214,7 @@
/* Various constants */
/* Coalescing */
-#define MVNETA_TXDONE_COAL_PKTS 16
+#define MVNETA_TXDONE_COAL_PKTS 1
#define MVNETA_RX_COAL_PKTS 32
#define MVNETA_RX_COAL_USEC 100
--
1.7.12.2.21.g234cd45.dirty
^ permalink raw reply related
* Re: Is this 32-bit NCM?
From: Eli Britstein @ 2014-12-02 7:11 UTC (permalink / raw)
To: Kevin Zhu
Cc: Enrico Mioso, Bjørn Mork, Alex Strizhevsky,
Midge Shaojun Tan, youtux@gmail.com, linux-usb@vger.kernel.org,
netdev@vger.kernel.org
In-Reply-To: <547D61C0.2080904@audiocodes.com>
Maybe the FW expects some kind of checksum or crc in the padding area?
Sent from my iPhone
> On 2 בדצמ 2014, at 08:52, "Kevin Zhu" <Mingying.Zhu@audiocodes.com> wrote:
>
> OK. I will look at the windows driver and registry. I don't have any
> manual of it.
>
> Regards,
> Kevin
>
>> On 12/02/2014 02:49 PM, Enrico Mioso wrote:
>> Can you try to look at Windows registry keys based on the INF file as suggested
>> or would this be a problem for you?
>> And... if you have any Huawei manutal talking about it: even device's
>> ^DSFLOWRPT
>> might be useful to some extent, but ... this is only just a note in case we
>> don't find anything else.
>> Regards ... and good day to all of you,
>> Enrico
>>
>>
>> On Tue, 2 Dec 2014, Kevin Zhu wrote:
>>
>> ==Date: Tue, 2 Dec 2014 07:43:24
>> ==From: Kevin Zhu <Mingying.Zhu@audiocodes.com>
>> ==To: Enrico Mioso <mrkiko.rs@gmail.com>
>> ==Cc: Eli Britstein <Eli.Britstein@audiocodes.com>, Bjørn Mork <bjorn@mork.no>,
>> == Alex Strizhevsky <alexxst@gmail.com>,
>> == Midge Shaojun Tan <ShaojunMidge.Tan@audiocodes.com>,
>> == "youtux@gmail.com" <youtux@gmail.com>,
>> == "linux-usb@vger.kernel.org" <linux-usb@vger.kernel.org>,
>> == "netdev@vger.kernel.org" <netdev@vger.kernel.org>
>> ==Subject: Re: Is this 32-bit NCM?
>> ==
>> ==I also tried to define CDC_NCM_DPT_DATAGRAMS_MAX as 1 to eliminate the
>> ==paddings, but it did not work either, not even DHCP.
>> ==
>> ==Regards,
>> ==Kevin
>> ==
>> ==On 12/02/2014 02:32 PM, Enrico Mioso wrote:
>> ==> Hi again Eli,
>> ==> Hi Kevin,
>> ==> Hi everyone...
>> ==>
>> ==> As I understand it anyway - the distinction here tends not to be between
>> ==> different types of packets.
>> ==> It tends to be between received and sent packet.
>> ==> We are not able to generate packets that the device retains valid.
>> ==> If you manually configure the IP the device would have assigned you via DHCP,
>> ==> your system will start answering ARP request, saying that the host with IP
>> ==> aa.bb.cc.dd is at aa:bb:cc:dd:ee (using the MC address of the dongle).
>> ==> However, the other host (gateway) will never know that. Indeed, it'll continue
>> ==> asking who-is at aa.bb.cc.dd ?
>> ==> At least, this is the situation I observed in the test system.
>> ==>
>> ==>
>> ==> On Tue, 2 Dec 2014, Kevin Zhu wrote:
>> ==>
>> ==> ==Date: Tue, 2 Dec 2014 04:53:53
>> ==> ==From: Kevin Zhu <Mingying.Zhu@audiocodes.com>
>> ==> ==To: Eli Britstein <Eli.Britstein@audiocodes.com>,
>> ==> == Enrico Mioso <mrkiko.rs@gmail.com>
>> ==> ==Cc: Bjørn Mork <bjorn@mork.no>, Alex Strizhevsky <alexxst@gmail.com>,
>> ==> == Midge Shaojun Tan <ShaojunMidge.Tan@audiocodes.com>,
>> ==> == "youtux@gmail.com" <youtux@gmail.com>,
>> ==> == "linux-usb@vger.kernel.org" <linux-usb@vger.kernel.org>,
>> ==> == "netdev@vger.kernel.org" <netdev@vger.kernel.org>
>> ==> ==Subject: Re: Is this 32-bit NCM?
>> ==> ==
>> ==> ==Hi,
>> ==> ==
>> ==> ==The DHCP packets have the maximum size of dwNtbOutMaxSize=16384, while
>> ==> ==the other packets are less than that. However, the DHCP queries are not
>> ==> ==replied in time either, there's always some delay.
>> ==> ==
>> ==> ==By the way, though the device claims to support GET_MAX_DATAGRAM_SIZE,
>> ==> ==but it returns error when the host sends this command to it. I disabled
>> ==> ==this command in NCM driver and tried, but it's the same result.
>> ==> ==
>> ==> ==Regards,
>> ==> ==Kevin
>> ==> ==
>> ==> ==On 12/02/2014 06:02 AM, Eli Britstein wrote:
>> ==> ==> Hi Enrico and all,
>> ==> ==>
>> ==> ==> Maybe I missed something but what is the difference by the driver point of view between the dhcp discover and request (that are ok) to others (like arp, that is nok)?
>> ==> ==> Maybe we can trace to compare them?
>> ==> ==>
>> ==> ==> Sent from my iPhone
>> ==> ==>
>> ==> ==>> On 1 בדצמ 2014, at 23:11, "Enrico Mioso" <mrkiko.rs@gmail.com> wrote:
>> ==> ==>>
>> ==> ==>> So ... I have two ideas left for now.
>> ==> ==>> We know for sure the problem is in the way we TX frames, not the way we RX them
>> ==> ==>> (the way we send, generate them, not the way we receive them).
>> ==> ==>> Si I have two ideas, and I ask for help from the Linux-usb mailing list for
>> ==> ==>> this first one.
>> ==> ==>>
>> ==> ==>> 1 - Does a wayexist to "replay" traffic crom a usb capture?
>> ==> ==>> We might try to take the usb frames generated by Windows, and send them to the
>> ==> ==>> device to see if there is any reaction. It should not be science fiction, I saw
>> ==> ==>> them do that in the eciadsl old project.
>> ==> ==>> 2 - The huawei ndis driver: does it work with these devices?
>> ==> ==>> It should be a little bit out-dated now (at least in terms of dates, it might
>> ==> ==>> work as well): the code is A LOT but, just in case, to see if there is any
>> ==> ==>> chances it'll work. It remains to be seen in which kernels it can actually
>> ==> ==>> compile again.
>> ==> ==>>
>> ==> ==>> If this works we might analyse what's happening and try to debug this out.
>> ==> ==>> But I really would like this to work in the cdc_ncm driver + huawei_cdc_ncm.
>> ==> ==>> Thank you.
>> ==> ==This email and any files transmitted with it are confidential material. They are intended solely for the use of the designated individual or entity to whom they are addressed. If the reader of this message is not the intended recipient, you are hereby notified that any dissemination, use, distribution or copying of this communication is strictly prohibited and may be unlawful.
>> ==> ==
>> ==> ==If you have received this email in error please immediately notify the sender and delete or destroy any copy of this message
>> ==> ==
>> ==This email and any files transmitted with it are confidential material. They are intended solely for the use of the designated individual or entity to whom they are addressed. If the reader of this message is not the intended recipient, you are hereby notified that any dissemination, use, distribution or copying of this communication is strictly prohibited and may be unlawful.
>> ==
>> ==If you have received this email in error please immediately notify the sender and delete or destroy any copy of this message
>> ==
________________________________
This email and any files transmitted with it are confidential material. They are intended solely for the use of the designated individual or entity to whom they are addressed. If the reader of this message is not the intended recipient, you are hereby notified that any dissemination, use, distribution or copying of this communication is strictly prohibited and may be unlawful.
If you have received this email in error please immediately notify the sender and delete or destroy any copy of this message
^ permalink raw reply
* [PATCH net-next 1/8] tipc: remove size variable from publ_list struct
From: Ying Xue @ 2014-12-02 7:00 UTC (permalink / raw)
To: davem
Cc: jon.maloy, Paul.Gortmaker, erik.hugne, richard.alpe, tero.aho,
netdev, tipc-discussion
In-Reply-To: <1417503630-31352-1-git-send-email-ying.xue@windriver.com>
The size variable is introduced in publ_list struct to help us exactly
calculate SKB buffer sizes needed by publications when all publications
in name table are delivered in bulk in named_distribute(). But if
publication SKB buffer size is assumed to MTU, the size variable in
publ_list struct can be completely eliminated at the cost of wasting
a bit memory space for last SKB.
Signed-off-by: Ying Xue <ying.xue@windriver.com>
Signed-off-by: Tero Aho <tero.aho@coriant.com>
Reviewed-by: Erik Hugne <erik.hugne@ericsson.com>
Reviewed-by: Jon Maloy <jon.maloy@ericsson.com>
Tested-by: Erik Hugne <erik.hugne@ericsson.com>
---
net/tipc/name_distr.c | 19 +++++++------------
1 file changed, 7 insertions(+), 12 deletions(-)
diff --git a/net/tipc/name_distr.c b/net/tipc/name_distr.c
index 56248db..628cd85 100644
--- a/net/tipc/name_distr.c
+++ b/net/tipc/name_distr.c
@@ -41,26 +41,21 @@
/**
* struct publ_list - list of publications made by this node
* @list: circular list of publications
- * @list_size: number of entries in list
*/
struct publ_list {
struct list_head list;
- u32 size;
};
static struct publ_list publ_zone = {
.list = LIST_HEAD_INIT(publ_zone.list),
- .size = 0,
};
static struct publ_list publ_cluster = {
.list = LIST_HEAD_INIT(publ_cluster.list),
- .size = 0,
};
static struct publ_list publ_node = {
.list = LIST_HEAD_INIT(publ_node.list),
- .size = 0,
};
static struct publ_list *publ_lists[] = {
@@ -147,7 +142,6 @@ struct sk_buff *tipc_named_publish(struct publication *publ)
struct distr_item *item;
list_add_tail(&publ->local_list, &publ_lists[publ->scope]->list);
- publ_lists[publ->scope]->size++;
if (publ->scope == TIPC_NODE_SCOPE)
return NULL;
@@ -172,7 +166,6 @@ struct sk_buff *tipc_named_withdraw(struct publication *publ)
struct distr_item *item;
list_del(&publ->local_list);
- publ_lists[publ->scope]->size--;
if (publ->scope == TIPC_NODE_SCOPE)
return NULL;
@@ -200,16 +193,12 @@ static void named_distribute(struct sk_buff_head *list, u32 dnode,
struct publication *publ;
struct sk_buff *skb = NULL;
struct distr_item *item = NULL;
- uint dsz = pls->size * ITEM_SIZE;
uint msg_dsz = (tipc_node_get_mtu(dnode, 0) / ITEM_SIZE) * ITEM_SIZE;
- uint rem = dsz;
- uint msg_rem = 0;
+ uint msg_rem = msg_dsz;
list_for_each_entry(publ, &pls->list, local_list) {
/* Prepare next buffer: */
if (!skb) {
- msg_rem = min_t(uint, rem, msg_dsz);
- rem -= msg_rem;
skb = named_prepare_buf(PUBLICATION, msg_rem, dnode);
if (!skb) {
pr_warn("Bulk publication failure\n");
@@ -227,8 +216,14 @@ static void named_distribute(struct sk_buff_head *list, u32 dnode,
if (!msg_rem) {
__skb_queue_tail(list, skb);
skb = NULL;
+ msg_rem = msg_dsz;
}
}
+ if (skb) {
+ msg_set_size(buf_msg(skb), INT_H_SIZE + (msg_dsz - msg_rem));
+ skb_trim(skb, INT_H_SIZE + (msg_dsz - msg_rem));
+ __skb_queue_tail(list, skb);
+ }
}
/**
--
1.7.9.5
^ permalink raw reply related
* [PATCH net-next 0/8] tipc: convert name table read-write lock to RCU
From: Ying Xue @ 2014-12-02 7:00 UTC (permalink / raw)
To: davem
Cc: jon.maloy, Paul.Gortmaker, erik.hugne, richard.alpe, tero.aho,
netdev, tipc-discussion
Now TIPC name table is statically allocated and is protected with a
Read-Write lock. To enhance the performance of TIPC name table lookup,
we are going to involve RCU lock to protect the name table. As a
consequence, it becomes lockless to concurrently look up name table on
read side. However, before the conversion can be successfully made,
the following two things must be first done:
- change allocation way of name table from static to dynamic
- fix several incorrect locking policy issues
Ying Xue (8):
tipc: remove size variable from publ_list struct
tipc: make name table allocated dynamically
tipc: ensure all name sequences are released when name table is
stopped
tipc: ensure all name sequences are properly protected with its lock
tipc: any name table member must be protected under name table lock
tipc: simplify relationship between name table lock and node lock
tipc: remove unnecessary INIT_LIST_HEAD
tipc: convert name table read-write lock to RCU
include/linux/rculist.h | 9 +++
net/tipc/name_distr.c | 83 +++++++------------
net/tipc/name_table.c | 202 ++++++++++++++++++++++++-----------------------
net/tipc/name_table.h | 20 ++++-
net/tipc/subscr.c | 1 -
5 files changed, 157 insertions(+), 158 deletions(-)
--
1.7.9.5
^ permalink raw reply
* [PATCH net-next 8/8] tipc: convert name table read-write lock to RCU
From: Ying Xue @ 2014-12-02 7:00 UTC (permalink / raw)
To: davem; +Cc: jon.maloy, netdev, Paul.Gortmaker, tipc-discussion
In-Reply-To: <1417503630-31352-1-git-send-email-ying.xue@windriver.com>
Convert tipc name table read-write lock to RCU. After this change,
a new spin lock is used to protect name table on write side while
RCU is applied on read side.
Signed-off-by: Ying Xue <ying.xue@windriver.com>
Reviewed-by: Erik Hugne <erik.hugne@ericsson.com>
Reviewed-by: Jon Maloy <jon.maloy@ericsson.com>
Tested-by: Erik Hugne <erik.hugne@ericsson.com>
---
include/linux/rculist.h | 9 +++++
net/tipc/name_distr.c | 28 +++++++--------
net/tipc/name_table.c | 87 +++++++++++++++++++++++------------------------
net/tipc/name_table.h | 4 ++-
4 files changed, 69 insertions(+), 59 deletions(-)
diff --git a/include/linux/rculist.h b/include/linux/rculist.h
index 372ad5e..aa79b3c 100644
--- a/include/linux/rculist.h
+++ b/include/linux/rculist.h
@@ -542,6 +542,15 @@ static inline void hlist_add_behind_rcu(struct hlist_node *n,
pos = hlist_entry_safe(rcu_dereference_bh((pos)->member.next),\
typeof(*(pos)), member))
+/**
+ * hlist_for_each_entry_from_rcu - iterate over a hlist continuing from current point
+ * @pos: the type * to use as a loop cursor.
+ * @member: the name of the hlist_node within the struct.
+ */
+#define hlist_for_each_entry_from_rcu(pos, member) \
+ for (; pos; \
+ pos = hlist_entry_safe(rcu_dereference((pos)->member.next),\
+ typeof(*(pos)), member))
#endif /* __KERNEL__ */
#endif
diff --git a/net/tipc/name_distr.c b/net/tipc/name_distr.c
index ed00929..ba6083d 100644
--- a/net/tipc/name_distr.c
+++ b/net/tipc/name_distr.c
@@ -113,8 +113,8 @@ struct sk_buff *tipc_named_publish(struct publication *publ)
struct sk_buff *buf;
struct distr_item *item;
- list_add_tail(&publ->local_list,
- &tipc_nametbl->publ_list[publ->scope]);
+ list_add_tail_rcu(&publ->local_list,
+ &tipc_nametbl->publ_list[publ->scope]);
if (publ->scope == TIPC_NODE_SCOPE)
return NULL;
@@ -208,12 +208,12 @@ void tipc_named_node_up(u32 dnode)
__skb_queue_head_init(&head);
- read_lock_bh(&tipc_nametbl_lock);
+ rcu_read_lock();
named_distribute(&head, dnode,
&tipc_nametbl->publ_list[TIPC_CLUSTER_SCOPE]);
named_distribute(&head, dnode,
&tipc_nametbl->publ_list[TIPC_ZONE_SCOPE]);
- read_unlock_bh(&tipc_nametbl_lock);
+ rcu_read_unlock();
tipc_link_xmit(&head, dnode, dnode);
}
@@ -260,12 +260,12 @@ static void tipc_publ_purge(struct publication *publ, u32 addr)
{
struct publication *p;
- write_lock_bh(&tipc_nametbl_lock);
+ spin_lock_bh(&tipc_nametbl_lock);
p = tipc_nametbl_remove_publ(publ->type, publ->lower,
publ->node, publ->ref, publ->key);
if (p)
tipc_publ_unsubscribe(p, addr);
- write_unlock_bh(&tipc_nametbl_lock);
+ spin_unlock_bh(&tipc_nametbl_lock);
if (p != publ) {
pr_err("Unable to remove publication from failed node\n"
@@ -274,7 +274,7 @@ static void tipc_publ_purge(struct publication *publ, u32 addr)
publ->key);
}
- kfree(p);
+ kfree_rcu(p, rcu);
}
void tipc_publ_notify(struct list_head *nsub_list, u32 addr)
@@ -311,7 +311,7 @@ static bool tipc_update_nametbl(struct distr_item *i, u32 node, u32 dtype)
ntohl(i->key));
if (publ) {
tipc_publ_unsubscribe(publ, node);
- kfree(publ);
+ kfree_rcu(publ, rcu);
return true;
}
} else {
@@ -376,14 +376,14 @@ void tipc_named_rcv(struct sk_buff *buf)
u32 count = msg_data_sz(msg) / ITEM_SIZE;
u32 node = msg_orignode(msg);
- write_lock_bh(&tipc_nametbl_lock);
+ spin_lock_bh(&tipc_nametbl_lock);
while (count--) {
if (!tipc_update_nametbl(item, node, msg_type(msg)))
tipc_named_add_backlog(item, msg_type(msg), node);
item++;
}
tipc_named_process_backlog();
- write_unlock_bh(&tipc_nametbl_lock);
+ spin_unlock_bh(&tipc_nametbl_lock);
kfree_skb(buf);
}
@@ -399,12 +399,12 @@ void tipc_named_reinit(void)
struct publication *publ;
int scope;
- write_lock_bh(&tipc_nametbl_lock);
+ spin_lock_bh(&tipc_nametbl_lock);
for (scope = TIPC_ZONE_SCOPE; scope <= TIPC_NODE_SCOPE; scope++)
- list_for_each_entry(publ, &tipc_nametbl->publ_list[scope],
- local_list)
+ list_for_each_entry_rcu(publ, &tipc_nametbl->publ_list[scope],
+ local_list)
publ->node = tipc_own_addr;
- write_unlock_bh(&tipc_nametbl_lock);
+ spin_unlock_bh(&tipc_nametbl_lock);
}
diff --git a/net/tipc/name_table.c b/net/tipc/name_table.c
index 3c2e0c3..aafa684c 100644
--- a/net/tipc/name_table.c
+++ b/net/tipc/name_table.c
@@ -92,6 +92,7 @@ struct sub_seq {
* @ns_list: links to adjacent name sequences in hash chain
* @subscriptions: list of subscriptions for this 'type'
* @lock: spinlock controlling access to publication lists of all sub-sequences
+ * @rcu: RCU callback head used for deferred freeing
*/
struct name_seq {
u32 type;
@@ -101,10 +102,11 @@ struct name_seq {
struct hlist_node ns_list;
struct list_head subscriptions;
spinlock_t lock;
+ struct rcu_head rcu;
};
struct name_table *tipc_nametbl;
-DEFINE_RWLOCK(tipc_nametbl_lock);
+DEFINE_SPINLOCK(tipc_nametbl_lock);
static int hash(int x)
{
@@ -166,7 +168,7 @@ static struct name_seq *tipc_nameseq_create(u32 type, struct hlist_head *seq_hea
nseq->alloc = 1;
INIT_HLIST_NODE(&nseq->ns_list);
INIT_LIST_HEAD(&nseq->subscriptions);
- hlist_add_head(&nseq->ns_list, seq_head);
+ hlist_add_head_rcu(&nseq->ns_list, seq_head);
return nseq;
}
@@ -451,7 +453,7 @@ static struct name_seq *nametbl_find_seq(u32 type)
struct name_seq *ns;
seq_head = &tipc_nametbl->seq_hlist[hash(type)];
- hlist_for_each_entry(ns, seq_head, ns_list) {
+ hlist_for_each_entry_rcu(ns, seq_head, ns_list) {
if (ns->type == type)
return ns;
}
@@ -498,10 +500,10 @@ struct publication *tipc_nametbl_remove_publ(u32 type, u32 lower,
spin_lock_bh(&seq->lock);
publ = tipc_nameseq_remove_publ(seq, lower, node, ref, key);
if (!seq->first_free && list_empty(&seq->subscriptions)) {
- hlist_del_init(&seq->ns_list);
- spin_unlock_bh(&seq->lock);
+ hlist_del_init_rcu(&seq->ns_list);
kfree(seq->sseqs);
- kfree(seq);
+ spin_unlock_bh(&seq->lock);
+ kfree_rcu(seq, rcu);
return publ;
}
spin_unlock_bh(&seq->lock);
@@ -533,7 +535,7 @@ u32 tipc_nametbl_translate(u32 type, u32 instance, u32 *destnode)
if (!tipc_in_scope(*destnode, tipc_own_addr))
return 0;
- read_lock_bh(&tipc_nametbl_lock);
+ rcu_read_lock();
seq = nametbl_find_seq(type);
if (unlikely(!seq))
goto not_found;
@@ -590,7 +592,7 @@ u32 tipc_nametbl_translate(u32 type, u32 instance, u32 *destnode)
no_match:
spin_unlock_bh(&seq->lock);
not_found:
- read_unlock_bh(&tipc_nametbl_lock);
+ rcu_read_unlock();
*destnode = node;
return ref;
}
@@ -616,7 +618,7 @@ int tipc_nametbl_mc_translate(u32 type, u32 lower, u32 upper, u32 limit,
struct name_info *info;
int res = 0;
- read_lock_bh(&tipc_nametbl_lock);
+ rcu_read_lock();
seq = nametbl_find_seq(type);
if (!seq)
goto exit;
@@ -641,7 +643,7 @@ int tipc_nametbl_mc_translate(u32 type, u32 lower, u32 upper, u32 limit,
}
spin_unlock_bh(&seq->lock);
exit:
- read_unlock_bh(&tipc_nametbl_lock);
+ rcu_read_unlock();
return res;
}
@@ -654,11 +656,11 @@ struct publication *tipc_nametbl_publish(u32 type, u32 lower, u32 upper,
struct publication *publ;
struct sk_buff *buf = NULL;
- write_lock_bh(&tipc_nametbl_lock);
+ spin_lock_bh(&tipc_nametbl_lock);
if (tipc_nametbl->local_publ_count >= TIPC_MAX_PUBLICATIONS) {
pr_warn("Publication failed, local publication limit reached (%u)\n",
TIPC_MAX_PUBLICATIONS);
- write_unlock_bh(&tipc_nametbl_lock);
+ spin_unlock_bh(&tipc_nametbl_lock);
return NULL;
}
@@ -670,7 +672,7 @@ struct publication *tipc_nametbl_publish(u32 type, u32 lower, u32 upper,
/* Any pending external events? */
tipc_named_process_backlog();
}
- write_unlock_bh(&tipc_nametbl_lock);
+ spin_unlock_bh(&tipc_nametbl_lock);
if (buf)
named_cluster_distribute(buf);
@@ -685,7 +687,7 @@ int tipc_nametbl_withdraw(u32 type, u32 lower, u32 ref, u32 key)
struct publication *publ;
struct sk_buff *skb = NULL;
- write_lock_bh(&tipc_nametbl_lock);
+ spin_lock_bh(&tipc_nametbl_lock);
publ = tipc_nametbl_remove_publ(type, lower, tipc_own_addr, ref, key);
if (likely(publ)) {
tipc_nametbl->local_publ_count--;
@@ -693,13 +695,13 @@ int tipc_nametbl_withdraw(u32 type, u32 lower, u32 ref, u32 key)
/* Any pending external events? */
tipc_named_process_backlog();
list_del_init(&publ->pport_list);
- kfree(publ);
+ kfree_rcu(publ, rcu);
} else {
pr_err("Unable to remove local publication\n"
"(type=%u, lower=%u, ref=%u, key=%u)\n",
type, lower, ref, key);
}
- write_unlock_bh(&tipc_nametbl_lock);
+ spin_unlock_bh(&tipc_nametbl_lock);
if (skb) {
named_cluster_distribute(skb);
@@ -717,7 +719,7 @@ void tipc_nametbl_subscribe(struct tipc_subscription *s)
int index = hash(type);
struct name_seq *seq;
- write_lock_bh(&tipc_nametbl_lock);
+ spin_lock_bh(&tipc_nametbl_lock);
seq = nametbl_find_seq(type);
if (!seq)
seq = tipc_nameseq_create(type,
@@ -730,7 +732,7 @@ void tipc_nametbl_subscribe(struct tipc_subscription *s)
pr_warn("Failed to create subscription for {%u,%u,%u}\n",
s->seq.type, s->seq.lower, s->seq.upper);
}
- write_unlock_bh(&tipc_nametbl_lock);
+ spin_unlock_bh(&tipc_nametbl_lock);
}
/**
@@ -740,24 +742,23 @@ void tipc_nametbl_unsubscribe(struct tipc_subscription *s)
{
struct name_seq *seq;
- write_lock_bh(&tipc_nametbl_lock);
+ spin_lock_bh(&tipc_nametbl_lock);
seq = nametbl_find_seq(s->seq.type);
if (seq != NULL) {
spin_lock_bh(&seq->lock);
list_del_init(&s->nameseq_list);
if (!seq->first_free && list_empty(&seq->subscriptions)) {
- hlist_del_init(&seq->ns_list);
- spin_unlock_bh(&seq->lock);
+ hlist_del_init_rcu(&seq->ns_list);
kfree(seq->sseqs);
- kfree(seq);
+ spin_unlock_bh(&seq->lock);
+ kfree_rcu(seq, rcu);
} else {
spin_unlock_bh(&seq->lock);
}
}
- write_unlock_bh(&tipc_nametbl_lock);
+ spin_unlock_bh(&tipc_nametbl_lock);
}
-
/**
* subseq_list - print specified sub-sequence contents into the given buffer
*/
@@ -880,7 +881,7 @@ static int nametbl_list(char *buf, int len, u32 depth_info,
upbound = ~0;
for (i = 0; i < TIPC_NAMETBL_SIZE; i++) {
seq_head = &tipc_nametbl->seq_hlist[i];
- hlist_for_each_entry(seq, seq_head, ns_list) {
+ hlist_for_each_entry_rcu(seq, seq_head, ns_list) {
ret += nameseq_list(seq, buf + ret, len - ret,
depth, seq->type,
lowbound, upbound, i);
@@ -896,7 +897,7 @@ static int nametbl_list(char *buf, int len, u32 depth_info,
ret += nametbl_header(buf + ret, len - ret, depth);
i = hash(type);
seq_head = &tipc_nametbl->seq_hlist[i];
- hlist_for_each_entry(seq, seq_head, ns_list) {
+ hlist_for_each_entry_rcu(seq, seq_head, ns_list) {
if (seq->type == type) {
ret += nameseq_list(seq, buf + ret, len - ret,
depth, type,
@@ -928,11 +929,11 @@ struct sk_buff *tipc_nametbl_get(const void *req_tlv_area, int req_tlv_space)
pb = TLV_DATA(rep_tlv);
pb_len = ULTRA_STRING_MAX_LEN;
argv = (struct tipc_name_table_query *)TLV_DATA(req_tlv_area);
- read_lock_bh(&tipc_nametbl_lock);
+ rcu_read_lock();
str_len = nametbl_list(pb, pb_len, ntohl(argv->depth),
ntohl(argv->type),
ntohl(argv->lowbound), ntohl(argv->upbound));
- read_unlock_bh(&tipc_nametbl_lock);
+ rcu_read_unlock();
str_len += 1; /* for "\0" */
skb_put(buf, TLV_SPACE(str_len));
TLV_SET(rep_tlv, TIPC_TLV_ULTRA_STRING, NULL, str_len);
@@ -974,13 +975,13 @@ static void tipc_purge_publications(struct name_seq *seq)
list_for_each_entry_safe(publ, safe, &info->zone_list, zone_list) {
tipc_nametbl_remove_publ(publ->type, publ->lower, publ->node,
publ->ref, publ->key);
- kfree(publ);
+ kfree_rcu(publ, rcu);
}
- hlist_del_init(&seq->ns_list);
+ hlist_del_init_rcu(&seq->ns_list);
+ kfree(seq->sseqs);
spin_lock_bh(&seq->lock);
- kfree(seq->sseqs);
- kfree(seq);
+ kfree_rcu(seq, rcu);
}
void tipc_nametbl_stop(void)
@@ -988,22 +989,22 @@ void tipc_nametbl_stop(void)
u32 i;
struct name_seq *seq;
struct hlist_head *seq_head;
- struct hlist_node *safe;
/* Verify name table is empty and purge any lingering
* publications, then release the name table
*/
- write_lock_bh(&tipc_nametbl_lock);
+ spin_lock_bh(&tipc_nametbl_lock);
for (i = 0; i < TIPC_NAMETBL_SIZE; i++) {
if (hlist_empty(&tipc_nametbl->seq_hlist[i]))
continue;
seq_head = &tipc_nametbl->seq_hlist[i];
- hlist_for_each_entry_safe(seq, safe, seq_head, ns_list) {
+ hlist_for_each_entry_rcu(seq, seq_head, ns_list) {
tipc_purge_publications(seq);
}
}
- write_unlock_bh(&tipc_nametbl_lock);
+ spin_unlock_bh(&tipc_nametbl_lock);
+ synchronize_net();
kfree(tipc_nametbl);
}
@@ -1109,7 +1110,7 @@ static int __tipc_nl_seq_list(struct tipc_nl_msg *msg, u32 *last_type,
u32 *last_lower, u32 *last_publ)
{
struct hlist_head *seq_head;
- struct name_seq *seq;
+ struct name_seq *seq = NULL;
int err;
int i;
@@ -1126,13 +1127,13 @@ static int __tipc_nl_seq_list(struct tipc_nl_msg *msg, u32 *last_type,
if (!seq)
return -EPIPE;
} else {
- seq = hlist_entry_safe((seq_head)->first,
- struct name_seq, ns_list);
+ hlist_for_each_entry_rcu(seq, seq_head, ns_list)
+ break;
if (!seq)
continue;
}
- hlist_for_each_entry_from(seq, ns_list) {
+ hlist_for_each_entry_from_rcu(seq, ns_list) {
spin_lock_bh(&seq->lock);
err = __tipc_nl_subseq_list(msg, seq, last_lower,
last_publ);
@@ -1165,8 +1166,7 @@ int tipc_nl_name_table_dump(struct sk_buff *skb, struct netlink_callback *cb)
msg.portid = NETLINK_CB(cb->skb).portid;
msg.seq = cb->nlh->nlmsg_seq;
- read_lock_bh(&tipc_nametbl_lock);
-
+ rcu_read_lock();
err = __tipc_nl_seq_list(&msg, &last_type, &last_lower, &last_publ);
if (!err) {
done = 1;
@@ -1179,8 +1179,7 @@ int tipc_nl_name_table_dump(struct sk_buff *skb, struct netlink_callback *cb)
*/
cb->prev_seq = 1;
}
-
- read_unlock_bh(&tipc_nametbl_lock);
+ rcu_read_unlock();
cb->args[0] = last_type;
cb->args[1] = last_lower;
diff --git a/net/tipc/name_table.h b/net/tipc/name_table.h
index c1fd734..5f0dee9 100644
--- a/net/tipc/name_table.h
+++ b/net/tipc/name_table.h
@@ -62,6 +62,7 @@ struct tipc_port_list;
* @node_list: adjacent matching name seq publications with >= node scope
* @cluster_list: adjacent matching name seq publications with >= cluster scope
* @zone_list: adjacent matching name seq publications with >= zone scope
+ * @rcu: RCU callback head used for deferred freeing
*
* Note that the node list, cluster list, and zone list are circular lists.
*/
@@ -79,6 +80,7 @@ struct publication {
struct list_head node_list;
struct list_head cluster_list;
struct list_head zone_list;
+ struct rcu_head rcu;
};
/**
@@ -93,7 +95,7 @@ struct name_table {
u32 local_publ_count;
};
-extern rwlock_t tipc_nametbl_lock;
+extern spinlock_t tipc_nametbl_lock;
extern struct name_table *tipc_nametbl;
int tipc_nl_name_table_dump(struct sk_buff *skb, struct netlink_callback *cb);
--
1.7.9.5
------------------------------------------------------------------------------
Download BIRT iHub F-Type - The Free Enterprise-Grade BIRT Server
from Actuate! Instantly Supercharge Your Business Reports and Dashboards
with Interactivity, Sharing, Native Excel Exports, App Integration & more
Get technology previously reserved for billion-dollar corporations, FREE
http://pubads.g.doubleclick.net/gampad/clk?id=157005751&iu=/4140/ostg.clktrk
^ permalink raw reply related
* [PATCH net-next 7/8] tipc: remove unnecessary INIT_LIST_HEAD
From: Ying Xue @ 2014-12-02 7:00 UTC (permalink / raw)
To: davem; +Cc: jon.maloy, netdev, Paul.Gortmaker, tipc-discussion
In-Reply-To: <1417503630-31352-1-git-send-email-ying.xue@windriver.com>
When a list_head variable is seen as a new entry to be added to a
list head, it's unnecessary to be initialized with INIT_LIST_HEAD().
Signed-off-by: Ying Xue <ying.xue@windriver.com>
Reviewed-by: Erik Hugne <erik.hugne@ericsson.com>
Reviewed-by: Jon Maloy <jon.maloy@ericsson.com>
Tested-by: Erik Hugne <erik.hugne@ericsson.com>
---
net/tipc/name_table.c | 2 --
net/tipc/subscr.c | 1 -
2 files changed, 3 deletions(-)
diff --git a/net/tipc/name_table.c b/net/tipc/name_table.c
index bc3e7ed..3c2e0c3 100644
--- a/net/tipc/name_table.c
+++ b/net/tipc/name_table.c
@@ -131,9 +131,7 @@ static struct publication *publ_create(u32 type, u32 lower, u32 upper,
publ->node = node;
publ->ref = port_ref;
publ->key = key;
- INIT_LIST_HEAD(&publ->local_list);
INIT_LIST_HEAD(&publ->pport_list);
- INIT_LIST_HEAD(&publ->nodesub_list);
return publ;
}
diff --git a/net/tipc/subscr.c b/net/tipc/subscr.c
index 31b5cb2..0344206 100644
--- a/net/tipc/subscr.c
+++ b/net/tipc/subscr.c
@@ -305,7 +305,6 @@ static int subscr_subscribe(struct tipc_subscr *s,
kfree(sub);
return -EINVAL;
}
- INIT_LIST_HEAD(&sub->nameseq_list);
list_add(&sub->subscription_list, &subscriber->subscription_list);
sub->subscriber = subscriber;
sub->swap = swap;
--
1.7.9.5
------------------------------------------------------------------------------
Download BIRT iHub F-Type - The Free Enterprise-Grade BIRT Server
from Actuate! Instantly Supercharge Your Business Reports and Dashboards
with Interactivity, Sharing, Native Excel Exports, App Integration & more
Get technology previously reserved for billion-dollar corporations, FREE
http://pubads.g.doubleclick.net/gampad/clk?id=157005751&iu=/4140/ostg.clktrk
^ permalink raw reply related
* [PATCH net-next 6/8] tipc: simplify relationship between name table lock and node lock
From: Ying Xue @ 2014-12-02 7:00 UTC (permalink / raw)
To: davem; +Cc: jon.maloy, netdev, Paul.Gortmaker, tipc-discussion
In-Reply-To: <1417503630-31352-1-git-send-email-ying.xue@windriver.com>
When tipc name sequence is published, name table lock is released
before name sequence buffer is delivered to remote nodes through its
underlying unicast links. However, when name sequence is withdrawn,
the name table lock is held until the transmission of the removal
message of name sequence is finished. During the process, node lock
is nested in name table lock. To prevent node lock from being nested
in name table lock, while withdrawing name, we should adopt the same
locking policy of publishing name sequence: name table lock should
be released before message is sent.
Signed-off-by: Ying Xue <ying.xue@windriver.com>
Reviewed-by: Erik Hugne <erik.hugne@ericsson.com>
Reviewed-by: Jon Maloy <jon.maloy@ericsson.com>
Tested-by: Erik Hugne <erik.hugne@ericsson.com>
---
net/tipc/name_table.c | 19 ++++++++++---------
1 file changed, 10 insertions(+), 9 deletions(-)
diff --git a/net/tipc/name_table.c b/net/tipc/name_table.c
index 93bac40..bc3e7ed 100644
--- a/net/tipc/name_table.c
+++ b/net/tipc/name_table.c
@@ -685,27 +685,28 @@ struct publication *tipc_nametbl_publish(u32 type, u32 lower, u32 upper,
int tipc_nametbl_withdraw(u32 type, u32 lower, u32 ref, u32 key)
{
struct publication *publ;
- struct sk_buff *buf;
+ struct sk_buff *skb = NULL;
write_lock_bh(&tipc_nametbl_lock);
publ = tipc_nametbl_remove_publ(type, lower, tipc_own_addr, ref, key);
if (likely(publ)) {
tipc_nametbl->local_publ_count--;
- buf = tipc_named_withdraw(publ);
+ skb = tipc_named_withdraw(publ);
/* Any pending external events? */
tipc_named_process_backlog();
- write_unlock_bh(&tipc_nametbl_lock);
list_del_init(&publ->pport_list);
kfree(publ);
+ } else {
+ pr_err("Unable to remove local publication\n"
+ "(type=%u, lower=%u, ref=%u, key=%u)\n",
+ type, lower, ref, key);
+ }
+ write_unlock_bh(&tipc_nametbl_lock);
- if (buf)
- named_cluster_distribute(buf);
+ if (skb) {
+ named_cluster_distribute(skb);
return 1;
}
- write_unlock_bh(&tipc_nametbl_lock);
- pr_err("Unable to remove local publication\n"
- "(type=%u, lower=%u, ref=%u, key=%u)\n",
- type, lower, ref, key);
return 0;
}
--
1.7.9.5
------------------------------------------------------------------------------
Download BIRT iHub F-Type - The Free Enterprise-Grade BIRT Server
from Actuate! Instantly Supercharge Your Business Reports and Dashboards
with Interactivity, Sharing, Native Excel Exports, App Integration & more
Get technology previously reserved for billion-dollar corporations, FREE
http://pubads.g.doubleclick.net/gampad/clk?id=157005751&iu=/4140/ostg.clktrk
^ permalink raw reply related
* [PATCH net-next 5/8] tipc: any name table member must be protected under name table lock
From: Ying Xue @ 2014-12-02 7:00 UTC (permalink / raw)
To: davem; +Cc: jon.maloy, netdev, Paul.Gortmaker, tipc-discussion
In-Reply-To: <1417503630-31352-1-git-send-email-ying.xue@windriver.com>
As tipc_nametbl_lock is used to protect name_table structure, the lock
must be held while all members of name_table structure are accessed.
However, the lock is not obtained while a member of name_table
structure - local_publ_count is read in tipc_nametbl_publish(), as
a consequence, an inconsistent value of local_publ_count might be got.
Signed-off-by: Ying Xue <ying.xue@windriver.com>
Reviewed-by: Erik Hugne <erik.hugne@ericsson.com>
Reviewed-by: Jon Maloy <jon.maloy@ericsson.com>
Tested-by: Erik Hugne <erik.hugne@ericsson.com>
---
net/tipc/name_table.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/net/tipc/name_table.c b/net/tipc/name_table.c
index 0d61f58..93bac40 100644
--- a/net/tipc/name_table.c
+++ b/net/tipc/name_table.c
@@ -656,13 +656,14 @@ struct publication *tipc_nametbl_publish(u32 type, u32 lower, u32 upper,
struct publication *publ;
struct sk_buff *buf = NULL;
+ write_lock_bh(&tipc_nametbl_lock);
if (tipc_nametbl->local_publ_count >= TIPC_MAX_PUBLICATIONS) {
pr_warn("Publication failed, local publication limit reached (%u)\n",
TIPC_MAX_PUBLICATIONS);
+ write_unlock_bh(&tipc_nametbl_lock);
return NULL;
}
- write_lock_bh(&tipc_nametbl_lock);
publ = tipc_nametbl_insert_publ(type, lower, upper, scope,
tipc_own_addr, port_ref, key);
if (likely(publ)) {
--
1.7.9.5
------------------------------------------------------------------------------
Download BIRT iHub F-Type - The Free Enterprise-Grade BIRT Server
from Actuate! Instantly Supercharge Your Business Reports and Dashboards
with Interactivity, Sharing, Native Excel Exports, App Integration & more
Get technology previously reserved for billion-dollar corporations, FREE
http://pubads.g.doubleclick.net/gampad/clk?id=157005751&iu=/4140/ostg.clktrk
^ permalink raw reply related
* [PATCH net-next 4/8] tipc: ensure all name sequences are properly protected with its lock
From: Ying Xue @ 2014-12-02 7:00 UTC (permalink / raw)
To: davem; +Cc: jon.maloy, netdev, Paul.Gortmaker, tipc-discussion
In-Reply-To: <1417503630-31352-1-git-send-email-ying.xue@windriver.com>
TIPC internally created a name table which is used to store name
sequences. Now there is a read-write lock - tipc_nametbl_lock to
protect the table, and each name sequence saved in the table is
protected with its private lock. When a name sequence is inserted
or removed to or from the table, its members might need to change.
Therefore, in normal case, the two locks must be held while TIPC
operates the table. However, there are still several places where
we only hold tipc_nametbl_lock without proprerly obtaining name
sequence lock, which might cause the corruption of name sequence.
Signed-off-by: Ying Xue <ying.xue@windriver.com>
Reviewed-by: Erik Hugne <erik.hugne@ericsson.com>
Reviewed-by: Jon Maloy <jon.maloy@ericsson.com>
Tested-by: Erik Hugne <erik.hugne@ericsson.com>
---
net/tipc/name_table.c | 48 +++++++++++++++++++++++++++---------------------
1 file changed, 27 insertions(+), 21 deletions(-)
diff --git a/net/tipc/name_table.c b/net/tipc/name_table.c
index ba0ee3e..0d61f58 100644
--- a/net/tipc/name_table.c
+++ b/net/tipc/name_table.c
@@ -172,18 +172,6 @@ static struct name_seq *tipc_nameseq_create(u32 type, struct hlist_head *seq_hea
return nseq;
}
-/*
- * nameseq_delete_empty - deletes a name sequence structure if now unused
- */
-static void nameseq_delete_empty(struct name_seq *seq)
-{
- if (!seq->first_free && list_empty(&seq->subscriptions)) {
- hlist_del_init(&seq->ns_list);
- kfree(seq->sseqs);
- kfree(seq);
- }
-}
-
/**
* nameseq_find_subseq - find sub-sequence (if any) matching a name instance
*
@@ -476,6 +464,7 @@ static struct name_seq *nametbl_find_seq(u32 type)
struct publication *tipc_nametbl_insert_publ(u32 type, u32 lower, u32 upper,
u32 scope, u32 node, u32 port, u32 key)
{
+ struct publication *publ;
struct name_seq *seq = nametbl_find_seq(type);
int index = hash(type);
@@ -492,8 +481,11 @@ struct publication *tipc_nametbl_insert_publ(u32 type, u32 lower, u32 upper,
if (!seq)
return NULL;
- return tipc_nameseq_insert_publ(seq, type, lower, upper,
+ spin_lock_bh(&seq->lock);
+ publ = tipc_nameseq_insert_publ(seq, type, lower, upper,
scope, node, port, key);
+ spin_unlock_bh(&seq->lock);
+ return publ;
}
struct publication *tipc_nametbl_remove_publ(u32 type, u32 lower,
@@ -505,8 +497,16 @@ struct publication *tipc_nametbl_remove_publ(u32 type, u32 lower,
if (!seq)
return NULL;
+ spin_lock_bh(&seq->lock);
publ = tipc_nameseq_remove_publ(seq, lower, node, ref, key);
- nameseq_delete_empty(seq);
+ if (!seq->first_free && list_empty(&seq->subscriptions)) {
+ hlist_del_init(&seq->ns_list);
+ spin_unlock_bh(&seq->lock);
+ kfree(seq->sseqs);
+ kfree(seq);
+ return publ;
+ }
+ spin_unlock_bh(&seq->lock);
return publ;
}
@@ -539,10 +539,10 @@ u32 tipc_nametbl_translate(u32 type, u32 instance, u32 *destnode)
seq = nametbl_find_seq(type);
if (unlikely(!seq))
goto not_found;
+ spin_lock_bh(&seq->lock);
sseq = nameseq_find_subseq(seq, instance);
if (unlikely(!sseq))
- goto not_found;
- spin_lock_bh(&seq->lock);
+ goto no_match;
info = sseq->info;
/* Closest-First Algorithm */
@@ -624,7 +624,6 @@ int tipc_nametbl_mc_translate(u32 type, u32 lower, u32 upper, u32 limit,
goto exit;
spin_lock_bh(&seq->lock);
-
sseq = seq->sseqs + nameseq_locate_subseq(seq, lower);
sseq_stop = seq->sseqs + seq->first_free;
for (; sseq != sseq_stop; sseq++) {
@@ -642,7 +641,6 @@ int tipc_nametbl_mc_translate(u32 type, u32 lower, u32 upper, u32 limit,
if (info->cluster_list_size != info->node_list_size)
res = 1;
}
-
spin_unlock_bh(&seq->lock);
exit:
read_unlock_bh(&tipc_nametbl_lock);
@@ -747,8 +745,14 @@ void tipc_nametbl_unsubscribe(struct tipc_subscription *s)
if (seq != NULL) {
spin_lock_bh(&seq->lock);
list_del_init(&s->nameseq_list);
- spin_unlock_bh(&seq->lock);
- nameseq_delete_empty(seq);
+ if (!seq->first_free && list_empty(&seq->subscriptions)) {
+ hlist_del_init(&seq->ns_list);
+ spin_unlock_bh(&seq->lock);
+ kfree(seq->sseqs);
+ kfree(seq);
+ } else {
+ spin_unlock_bh(&seq->lock);
+ }
}
write_unlock_bh(&tipc_nametbl_lock);
}
@@ -964,6 +968,7 @@ static void tipc_purge_publications(struct name_seq *seq)
struct sub_seq *sseq;
struct name_info *info;
+ spin_lock_bh(&seq->lock);
sseq = seq->sseqs;
info = sseq->info;
list_for_each_entry_safe(publ, safe, &info->zone_list, zone_list) {
@@ -972,6 +977,8 @@ static void tipc_purge_publications(struct name_seq *seq)
kfree(publ);
}
hlist_del_init(&seq->ns_list);
+ spin_lock_bh(&seq->lock);
+
kfree(seq->sseqs);
kfree(seq);
}
@@ -1127,7 +1134,6 @@ static int __tipc_nl_seq_list(struct tipc_nl_msg *msg, u32 *last_type,
hlist_for_each_entry_from(seq, ns_list) {
spin_lock_bh(&seq->lock);
-
err = __tipc_nl_subseq_list(msg, seq, last_lower,
last_publ);
--
1.7.9.5
------------------------------------------------------------------------------
Download BIRT iHub F-Type - The Free Enterprise-Grade BIRT Server
from Actuate! Instantly Supercharge Your Business Reports and Dashboards
with Interactivity, Sharing, Native Excel Exports, App Integration & more
Get technology previously reserved for billion-dollar corporations, FREE
http://pubads.g.doubleclick.net/gampad/clk?id=157005751&iu=/4140/ostg.clktrk
^ permalink raw reply related
* [PATCH net-next 3/8] tipc: ensure all name sequences are released when name table is stopped
From: Ying Xue @ 2014-12-02 7:00 UTC (permalink / raw)
To: davem; +Cc: jon.maloy, netdev, Paul.Gortmaker, tipc-discussion
In-Reply-To: <1417503630-31352-1-git-send-email-ying.xue@windriver.com>
As TIPC subscriber server is terminated before name table, no user
depends on subscription list of name sequence when name table is
stopped. Therefore, all name sequences stored in name table should
be released whatever their subscriptions lists are empty or not,
otherwise, memory leak might happen.
Signed-off-by: Ying Xue <ying.xue@windriver.com>
Reviewed-by: Erik Hugne <erik.hugne@ericsson.com>
Reviewed-by: Jon Maloy <jon.maloy@ericsson.com>
Tested-by: Erik Hugne <erik.hugne@ericsson.com>
---
net/tipc/name_table.c | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/net/tipc/name_table.c b/net/tipc/name_table.c
index df3da29..ba0ee3e 100644
--- a/net/tipc/name_table.c
+++ b/net/tipc/name_table.c
@@ -964,10 +964,6 @@ static void tipc_purge_publications(struct name_seq *seq)
struct sub_seq *sseq;
struct name_info *info;
- if (!seq->sseqs) {
- nameseq_delete_empty(seq);
- return;
- }
sseq = seq->sseqs;
info = sseq->info;
list_for_each_entry_safe(publ, safe, &info->zone_list, zone_list) {
@@ -975,6 +971,9 @@ static void tipc_purge_publications(struct name_seq *seq)
publ->ref, publ->key);
kfree(publ);
}
+ hlist_del_init(&seq->ns_list);
+ kfree(seq->sseqs);
+ kfree(seq);
}
void tipc_nametbl_stop(void)
--
1.7.9.5
------------------------------------------------------------------------------
Download BIRT iHub F-Type - The Free Enterprise-Grade BIRT Server
from Actuate! Instantly Supercharge Your Business Reports and Dashboards
with Interactivity, Sharing, Native Excel Exports, App Integration & more
Get technology previously reserved for billion-dollar corporations, FREE
http://pubads.g.doubleclick.net/gampad/clk?id=157005751&iu=/4140/ostg.clktrk
^ permalink raw reply related
* [PATCH net-next 2/8] tipc: make name table allocated dynamically
From: Ying Xue @ 2014-12-02 7:00 UTC (permalink / raw)
To: davem; +Cc: jon.maloy, netdev, Paul.Gortmaker, tipc-discussion
In-Reply-To: <1417503630-31352-1-git-send-email-ying.xue@windriver.com>
Name table locking policy is going to be adjusted from read-write
lock protection to RCU lock protection in the future commits. But
its essential precondition is to convert the allocation way of name
table from static to dynamic mode.
Signed-off-by: Ying Xue <ying.xue@windriver.com>
Reviewed-by: Erik Hugne <erik.hugne@ericsson.com>
Reviewed-by: Jon Maloy <jon.maloy@ericsson.com>
Tested-by: Erik Hugne <erik.hugne@ericsson.com>
---
net/tipc/name_distr.c | 44 +++++++++---------------------------
net/tipc/name_table.c | 60 ++++++++++++++++++++++++-------------------------
net/tipc/name_table.h | 16 ++++++++++++-
3 files changed, 55 insertions(+), 65 deletions(-)
diff --git a/net/tipc/name_distr.c b/net/tipc/name_distr.c
index 628cd85..ed00929 100644
--- a/net/tipc/name_distr.c
+++ b/net/tipc/name_distr.c
@@ -38,34 +38,6 @@
#include "link.h"
#include "name_distr.h"
-/**
- * struct publ_list - list of publications made by this node
- * @list: circular list of publications
- */
-struct publ_list {
- struct list_head list;
-};
-
-static struct publ_list publ_zone = {
- .list = LIST_HEAD_INIT(publ_zone.list),
-};
-
-static struct publ_list publ_cluster = {
- .list = LIST_HEAD_INIT(publ_cluster.list),
-};
-
-static struct publ_list publ_node = {
- .list = LIST_HEAD_INIT(publ_node.list),
-};
-
-static struct publ_list *publ_lists[] = {
- NULL,
- &publ_zone, /* publ_lists[TIPC_ZONE_SCOPE] */
- &publ_cluster, /* publ_lists[TIPC_CLUSTER_SCOPE] */
- &publ_node /* publ_lists[TIPC_NODE_SCOPE] */
-};
-
-
int sysctl_tipc_named_timeout __read_mostly = 2000;
/**
@@ -141,7 +113,8 @@ struct sk_buff *tipc_named_publish(struct publication *publ)
struct sk_buff *buf;
struct distr_item *item;
- list_add_tail(&publ->local_list, &publ_lists[publ->scope]->list);
+ list_add_tail(&publ->local_list,
+ &tipc_nametbl->publ_list[publ->scope]);
if (publ->scope == TIPC_NODE_SCOPE)
return NULL;
@@ -188,7 +161,7 @@ struct sk_buff *tipc_named_withdraw(struct publication *publ)
* @pls: linked list of publication items to be packed into buffer chain
*/
static void named_distribute(struct sk_buff_head *list, u32 dnode,
- struct publ_list *pls)
+ struct list_head *pls)
{
struct publication *publ;
struct sk_buff *skb = NULL;
@@ -196,7 +169,7 @@ static void named_distribute(struct sk_buff_head *list, u32 dnode,
uint msg_dsz = (tipc_node_get_mtu(dnode, 0) / ITEM_SIZE) * ITEM_SIZE;
uint msg_rem = msg_dsz;
- list_for_each_entry(publ, &pls->list, local_list) {
+ list_for_each_entry(publ, pls, local_list) {
/* Prepare next buffer: */
if (!skb) {
skb = named_prepare_buf(PUBLICATION, msg_rem, dnode);
@@ -236,8 +209,10 @@ void tipc_named_node_up(u32 dnode)
__skb_queue_head_init(&head);
read_lock_bh(&tipc_nametbl_lock);
- named_distribute(&head, dnode, &publ_cluster);
- named_distribute(&head, dnode, &publ_zone);
+ named_distribute(&head, dnode,
+ &tipc_nametbl->publ_list[TIPC_CLUSTER_SCOPE]);
+ named_distribute(&head, dnode,
+ &tipc_nametbl->publ_list[TIPC_ZONE_SCOPE]);
read_unlock_bh(&tipc_nametbl_lock);
tipc_link_xmit(&head, dnode, dnode);
@@ -427,7 +402,8 @@ void tipc_named_reinit(void)
write_lock_bh(&tipc_nametbl_lock);
for (scope = TIPC_ZONE_SCOPE; scope <= TIPC_NODE_SCOPE; scope++)
- list_for_each_entry(publ, &publ_lists[scope]->list, local_list)
+ list_for_each_entry(publ, &tipc_nametbl->publ_list[scope],
+ local_list)
publ->node = tipc_own_addr;
write_unlock_bh(&tipc_nametbl_lock);
diff --git a/net/tipc/name_table.c b/net/tipc/name_table.c
index 772be1c..df3da29 100644
--- a/net/tipc/name_table.c
+++ b/net/tipc/name_table.c
@@ -2,7 +2,7 @@
* net/tipc/name_table.c: TIPC name table code
*
* Copyright (c) 2000-2006, 2014, Ericsson AB
- * Copyright (c) 2004-2008, 2010-2011, Wind River Systems
+ * Copyright (c) 2004-2008, 2010-2014, Wind River Systems
* All rights reserved.
*
* Redistribution and use in source and binary forms, with or without
@@ -103,18 +103,7 @@ struct name_seq {
spinlock_t lock;
};
-/**
- * struct name_table - table containing all existing port name publications
- * @types: pointer to fixed-sized array of name sequence lists,
- * accessed via hashing on 'type'; name sequence lists are *not* sorted
- * @local_publ_count: number of publications issued by this node
- */
-struct name_table {
- struct hlist_head *types;
- u32 local_publ_count;
-};
-
-static struct name_table table;
+struct name_table *tipc_nametbl;
DEFINE_RWLOCK(tipc_nametbl_lock);
static int hash(int x)
@@ -475,7 +464,7 @@ static struct name_seq *nametbl_find_seq(u32 type)
struct hlist_head *seq_head;
struct name_seq *ns;
- seq_head = &table.types[hash(type)];
+ seq_head = &tipc_nametbl->seq_hlist[hash(type)];
hlist_for_each_entry(ns, seq_head, ns_list) {
if (ns->type == type)
return ns;
@@ -488,6 +477,7 @@ struct publication *tipc_nametbl_insert_publ(u32 type, u32 lower, u32 upper,
u32 scope, u32 node, u32 port, u32 key)
{
struct name_seq *seq = nametbl_find_seq(type);
+ int index = hash(type);
if ((scope < TIPC_ZONE_SCOPE) || (scope > TIPC_NODE_SCOPE) ||
(lower > upper)) {
@@ -497,7 +487,8 @@ struct publication *tipc_nametbl_insert_publ(u32 type, u32 lower, u32 upper,
}
if (!seq)
- seq = tipc_nameseq_create(type, &table.types[hash(type)]);
+ seq = tipc_nameseq_create(type,
+ &tipc_nametbl->seq_hlist[index]);
if (!seq)
return NULL;
@@ -667,7 +658,7 @@ struct publication *tipc_nametbl_publish(u32 type, u32 lower, u32 upper,
struct publication *publ;
struct sk_buff *buf = NULL;
- if (table.local_publ_count >= TIPC_MAX_PUBLICATIONS) {
+ if (tipc_nametbl->local_publ_count >= TIPC_MAX_PUBLICATIONS) {
pr_warn("Publication failed, local publication limit reached (%u)\n",
TIPC_MAX_PUBLICATIONS);
return NULL;
@@ -677,7 +668,7 @@ struct publication *tipc_nametbl_publish(u32 type, u32 lower, u32 upper,
publ = tipc_nametbl_insert_publ(type, lower, upper, scope,
tipc_own_addr, port_ref, key);
if (likely(publ)) {
- table.local_publ_count++;
+ tipc_nametbl->local_publ_count++;
buf = tipc_named_publish(publ);
/* Any pending external events? */
tipc_named_process_backlog();
@@ -700,7 +691,7 @@ int tipc_nametbl_withdraw(u32 type, u32 lower, u32 ref, u32 key)
write_lock_bh(&tipc_nametbl_lock);
publ = tipc_nametbl_remove_publ(type, lower, tipc_own_addr, ref, key);
if (likely(publ)) {
- table.local_publ_count--;
+ tipc_nametbl->local_publ_count--;
buf = tipc_named_withdraw(publ);
/* Any pending external events? */
tipc_named_process_backlog();
@@ -725,12 +716,14 @@ int tipc_nametbl_withdraw(u32 type, u32 lower, u32 ref, u32 key)
void tipc_nametbl_subscribe(struct tipc_subscription *s)
{
u32 type = s->seq.type;
+ int index = hash(type);
struct name_seq *seq;
write_lock_bh(&tipc_nametbl_lock);
seq = nametbl_find_seq(type);
if (!seq)
- seq = tipc_nameseq_create(type, &table.types[hash(type)]);
+ seq = tipc_nameseq_create(type,
+ &tipc_nametbl->seq_hlist[index]);
if (seq) {
spin_lock_bh(&seq->lock);
tipc_nameseq_subscribe(seq, s);
@@ -882,7 +875,7 @@ static int nametbl_list(char *buf, int len, u32 depth_info,
lowbound = 0;
upbound = ~0;
for (i = 0; i < TIPC_NAMETBL_SIZE; i++) {
- seq_head = &table.types[i];
+ seq_head = &tipc_nametbl->seq_hlist[i];
hlist_for_each_entry(seq, seq_head, ns_list) {
ret += nameseq_list(seq, buf + ret, len - ret,
depth, seq->type,
@@ -898,7 +891,7 @@ static int nametbl_list(char *buf, int len, u32 depth_info,
}
ret += nametbl_header(buf + ret, len - ret, depth);
i = hash(type);
- seq_head = &table.types[i];
+ seq_head = &tipc_nametbl->seq_hlist[i];
hlist_for_each_entry(seq, seq_head, ns_list) {
if (seq->type == type) {
ret += nameseq_list(seq, buf + ret, len - ret,
@@ -945,12 +938,18 @@ struct sk_buff *tipc_nametbl_get(const void *req_tlv_area, int req_tlv_space)
int tipc_nametbl_init(void)
{
- table.types = kcalloc(TIPC_NAMETBL_SIZE, sizeof(struct hlist_head),
- GFP_ATOMIC);
- if (!table.types)
+ int i;
+
+ tipc_nametbl = kzalloc(sizeof(*tipc_nametbl), GFP_ATOMIC);
+ if (!tipc_nametbl)
return -ENOMEM;
- table.local_publ_count = 0;
+ for (i = 0; i < TIPC_NAMETBL_SIZE; i++)
+ INIT_HLIST_HEAD(&tipc_nametbl->seq_hlist[i]);
+
+ INIT_LIST_HEAD(&tipc_nametbl->publ_list[TIPC_ZONE_SCOPE]);
+ INIT_LIST_HEAD(&tipc_nametbl->publ_list[TIPC_CLUSTER_SCOPE]);
+ INIT_LIST_HEAD(&tipc_nametbl->publ_list[TIPC_NODE_SCOPE]);
return 0;
}
@@ -990,16 +989,17 @@ void tipc_nametbl_stop(void)
*/
write_lock_bh(&tipc_nametbl_lock);
for (i = 0; i < TIPC_NAMETBL_SIZE; i++) {
- if (hlist_empty(&table.types[i]))
+ if (hlist_empty(&tipc_nametbl->seq_hlist[i]))
continue;
- seq_head = &table.types[i];
+ seq_head = &tipc_nametbl->seq_hlist[i];
hlist_for_each_entry_safe(seq, safe, seq_head, ns_list) {
tipc_purge_publications(seq);
}
}
- kfree(table.types);
- table.types = NULL;
write_unlock_bh(&tipc_nametbl_lock);
+
+ kfree(tipc_nametbl);
+
}
static int __tipc_nl_add_nametable_publ(struct tipc_nl_msg *msg,
@@ -1113,7 +1113,7 @@ static int __tipc_nl_seq_list(struct tipc_nl_msg *msg, u32 *last_type,
i = 0;
for (; i < TIPC_NAMETBL_SIZE; i++) {
- seq_head = &table.types[i];
+ seq_head = &tipc_nametbl->seq_hlist[i];
if (*last_type) {
seq = nametbl_find_seq(*last_type);
diff --git a/net/tipc/name_table.h b/net/tipc/name_table.h
index c628778..c1fd734 100644
--- a/net/tipc/name_table.h
+++ b/net/tipc/name_table.h
@@ -43,7 +43,9 @@ struct tipc_port_list;
/*
* TIPC name types reserved for internal TIPC use (both current and planned)
*/
-#define TIPC_ZM_SRV 3 /* zone master service name type */
+#define TIPC_ZM_SRV 3 /* zone master service name type */
+#define TIPC_PUBL_SCOPE_NUM (TIPC_NODE_SCOPE + 1)
+#define TIPC_NAMETBL_SIZE 1024 /* must be a power of 2 */
/**
* struct publication - info about a published (name or) name sequence
@@ -79,8 +81,20 @@ struct publication {
struct list_head zone_list;
};
+/**
+ * struct name_table - table containing all existing port name publications
+ * @seq_hlist: name sequence hash lists
+ * @publ_list: pulication lists
+ * @local_publ_count: number of publications issued by this node
+ */
+struct name_table {
+ struct hlist_head seq_hlist[TIPC_NAMETBL_SIZE];
+ struct list_head publ_list[TIPC_PUBL_SCOPE_NUM];
+ u32 local_publ_count;
+};
extern rwlock_t tipc_nametbl_lock;
+extern struct name_table *tipc_nametbl;
int tipc_nl_name_table_dump(struct sk_buff *skb, struct netlink_callback *cb);
--
1.7.9.5
------------------------------------------------------------------------------
Download BIRT iHub F-Type - The Free Enterprise-Grade BIRT Server
from Actuate! Instantly Supercharge Your Business Reports and Dashboards
with Interactivity, Sharing, Native Excel Exports, App Integration & more
Get technology previously reserved for billion-dollar corporations, FREE
http://pubads.g.doubleclick.net/gampad/clk?id=157005751&iu=/4140/ostg.clktrk
^ permalink raw reply related
* Re: Is this 32-bit NCM?
From: Kevin Zhu @ 2014-12-02 6:52 UTC (permalink / raw)
To: Enrico Mioso
Cc: Eli Britstein, Bjørn Mork, Alex Strizhevsky,
Midge Shaojun Tan, youtux-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org,
linux-usb-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <alpine.LNX.2.03.1412020747360.1921-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
OK. I will look at the windows driver and registry. I don't have any
manual of it.
Regards,
Kevin
On 12/02/2014 02:49 PM, Enrico Mioso wrote:
> Can you try to look at Windows registry keys based on the INF file as suggested
> or would this be a problem for you?
> And... if you have any Huawei manutal talking about it: even device's
> ^DSFLOWRPT
> might be useful to some extent, but ... this is only just a note in case we
> don't find anything else.
> Regards ... and good day to all of you,
> Enrico
>
>
> On Tue, 2 Dec 2014, Kevin Zhu wrote:
>
> ==Date: Tue, 2 Dec 2014 07:43:24
> ==From: Kevin Zhu <Mingying.Zhu@audiocodes.com>
> ==To: Enrico Mioso <mrkiko.rs@gmail.com>
> ==Cc: Eli Britstein <Eli.Britstein@audiocodes.com>, Bjørn Mork <bjorn@mork.no>,
> == Alex Strizhevsky <alexxst@gmail.com>,
> == Midge Shaojun Tan <ShaojunMidge.Tan@audiocodes.com>,
> == "youtux@gmail.com" <youtux@gmail.com>,
> == "linux-usb@vger.kernel.org" <linux-usb@vger.kernel.org>,
> == "netdev@vger.kernel.org" <netdev@vger.kernel.org>
> ==Subject: Re: Is this 32-bit NCM?
> ==
> ==I also tried to define CDC_NCM_DPT_DATAGRAMS_MAX as 1 to eliminate the
> ==paddings, but it did not work either, not even DHCP.
> ==
> ==Regards,
> ==Kevin
> ==
> ==On 12/02/2014 02:32 PM, Enrico Mioso wrote:
> ==> Hi again Eli,
> ==> Hi Kevin,
> ==> Hi everyone...
> ==>
> ==> As I understand it anyway - the distinction here tends not to be between
> ==> different types of packets.
> ==> It tends to be between received and sent packet.
> ==> We are not able to generate packets that the device retains valid.
> ==> If you manually configure the IP the device would have assigned you via DHCP,
> ==> your system will start answering ARP request, saying that the host with IP
> ==> aa.bb.cc.dd is at aa:bb:cc:dd:ee (using the MC address of the dongle).
> ==> However, the other host (gateway) will never know that. Indeed, it'll continue
> ==> asking who-is at aa.bb.cc.dd ?
> ==> At least, this is the situation I observed in the test system.
> ==>
> ==>
> ==> On Tue, 2 Dec 2014, Kevin Zhu wrote:
> ==>
> ==> ==Date: Tue, 2 Dec 2014 04:53:53
> ==> ==From: Kevin Zhu <Mingying.Zhu@audiocodes.com>
> ==> ==To: Eli Britstein <Eli.Britstein@audiocodes.com>,
> ==> == Enrico Mioso <mrkiko.rs@gmail.com>
> ==> ==Cc: Bjørn Mork <bjorn@mork.no>, Alex Strizhevsky <alexxst@gmail.com>,
> ==> == Midge Shaojun Tan <ShaojunMidge.Tan@audiocodes.com>,
> ==> == "youtux@gmail.com" <youtux@gmail.com>,
> ==> == "linux-usb@vger.kernel.org" <linux-usb@vger.kernel.org>,
> ==> == "netdev@vger.kernel.org" <netdev@vger.kernel.org>
> ==> ==Subject: Re: Is this 32-bit NCM?
> ==> ==
> ==> ==Hi,
> ==> ==
> ==> ==The DHCP packets have the maximum size of dwNtbOutMaxSize=16384, while
> ==> ==the other packets are less than that. However, the DHCP queries are not
> ==> ==replied in time either, there's always some delay.
> ==> ==
> ==> ==By the way, though the device claims to support GET_MAX_DATAGRAM_SIZE,
> ==> ==but it returns error when the host sends this command to it. I disabled
> ==> ==this command in NCM driver and tried, but it's the same result.
> ==> ==
> ==> ==Regards,
> ==> ==Kevin
> ==> ==
> ==> ==On 12/02/2014 06:02 AM, Eli Britstein wrote:
> ==> ==> Hi Enrico and all,
> ==> ==>
> ==> ==> Maybe I missed something but what is the difference by the driver point of view between the dhcp discover and request (that are ok) to others (like arp, that is nok)?
> ==> ==> Maybe we can trace to compare them?
> ==> ==>
> ==> ==> Sent from my iPhone
> ==> ==>
> ==> ==>> On 1 בדצמ 2014, at 23:11, "Enrico Mioso" <mrkiko.rs@gmail.com> wrote:
> ==> ==>>
> ==> ==>> So ... I have two ideas left for now.
> ==> ==>> We know for sure the problem is in the way we TX frames, not the way we RX them
> ==> ==>> (the way we send, generate them, not the way we receive them).
> ==> ==>> Si I have two ideas, and I ask for help from the Linux-usb mailing list for
> ==> ==>> this first one.
> ==> ==>>
> ==> ==>> 1 - Does a wayexist to "replay" traffic crom a usb capture?
> ==> ==>> We might try to take the usb frames generated by Windows, and send them to the
> ==> ==>> device to see if there is any reaction. It should not be science fiction, I saw
> ==> ==>> them do that in the eciadsl old project.
> ==> ==>> 2 - The huawei ndis driver: does it work with these devices?
> ==> ==>> It should be a little bit out-dated now (at least in terms of dates, it might
> ==> ==>> work as well): the code is A LOT but, just in case, to see if there is any
> ==> ==>> chances it'll work. It remains to be seen in which kernels it can actually
> ==> ==>> compile again.
> ==> ==>>
> ==> ==>> If this works we might analyse what's happening and try to debug this out.
> ==> ==>> But I really would like this to work in the cdc_ncm driver + huawei_cdc_ncm.
> ==> ==>> Thank you.
> ==> ==This email and any files transmitted with it are confidential material. They are intended solely for the use of the designated individual or entity to whom they are addressed. If the reader of this message is not the intended recipient, you are hereby notified that any dissemination, use, distribution or copying of this communication is strictly prohibited and may be unlawful.
> ==> ==
> ==> ==If you have received this email in error please immediately notify the sender and delete or destroy any copy of this message
> ==> ==
> ==This email and any files transmitted with it are confidential material. They are intended solely for the use of the designated individual or entity to whom they are addressed. If the reader of this message is not the intended recipient, you are hereby notified that any dissemination, use, distribution or copying of this communication is strictly prohibited and may be unlawful.
> ==
> ==If you have received this email in error please immediately notify the sender and delete or destroy any copy of this message
> ==
This email and any files transmitted with it are confidential material. They are intended solely for the use of the designated individual or entity to whom they are addressed. If the reader of this message is not the intended recipient, you are hereby notified that any dissemination, use, distribution or copying of this communication is strictly prohibited and may be unlawful.
If you have received this email in error please immediately notify the sender and delete or destroy any copy of this message
^ permalink raw reply
* Re: Is this 32-bit NCM?
From: Enrico Mioso @ 2014-12-02 6:49 UTC (permalink / raw)
To: Kevin Zhu
Cc: Eli Britstein, Bjørn Mork, Alex Strizhevsky,
Midge Shaojun Tan, youtux@gmail.com, linux-usb@vger.kernel.org,
netdev@vger.kernel.org
In-Reply-To: <547D5F84.6020608@audiocodes.com>
[-- Attachment #1: Type: TEXT/PLAIN, Size: 5571 bytes --]
Can you try to look at Windows registry keys based on the INF file as suggested
or would this be a problem for you?
And... if you have any Huawei manutal talking about it: even device's
^DSFLOWRPT
might be useful to some extent, but ... this is only just a note in case we
don't find anything else.
Regards ... and good day to all of you,
Enrico
On Tue, 2 Dec 2014, Kevin Zhu wrote:
==Date: Tue, 2 Dec 2014 07:43:24
==From: Kevin Zhu <Mingying.Zhu@audiocodes.com>
==To: Enrico Mioso <mrkiko.rs@gmail.com>
==Cc: Eli Britstein <Eli.Britstein@audiocodes.com>, Bjørn Mork <bjorn@mork.no>,
== Alex Strizhevsky <alexxst@gmail.com>,
== Midge Shaojun Tan <ShaojunMidge.Tan@audiocodes.com>,
== "youtux@gmail.com" <youtux@gmail.com>,
== "linux-usb@vger.kernel.org" <linux-usb@vger.kernel.org>,
== "netdev@vger.kernel.org" <netdev@vger.kernel.org>
==Subject: Re: Is this 32-bit NCM?
==
==I also tried to define CDC_NCM_DPT_DATAGRAMS_MAX as 1 to eliminate the
==paddings, but it did not work either, not even DHCP.
==
==Regards,
==Kevin
==
==On 12/02/2014 02:32 PM, Enrico Mioso wrote:
==> Hi again Eli,
==> Hi Kevin,
==> Hi everyone...
==>
==> As I understand it anyway - the distinction here tends not to be between
==> different types of packets.
==> It tends to be between received and sent packet.
==> We are not able to generate packets that the device retains valid.
==> If you manually configure the IP the device would have assigned you via DHCP,
==> your system will start answering ARP request, saying that the host with IP
==> aa.bb.cc.dd is at aa:bb:cc:dd:ee (using the MC address of the dongle).
==> However, the other host (gateway) will never know that. Indeed, it'll continue
==> asking who-is at aa.bb.cc.dd ?
==> At least, this is the situation I observed in the test system.
==>
==>
==> On Tue, 2 Dec 2014, Kevin Zhu wrote:
==>
==> ==Date: Tue, 2 Dec 2014 04:53:53
==> ==From: Kevin Zhu <Mingying.Zhu@audiocodes.com>
==> ==To: Eli Britstein <Eli.Britstein@audiocodes.com>,
==> == Enrico Mioso <mrkiko.rs@gmail.com>
==> ==Cc: Bjørn Mork <bjorn@mork.no>, Alex Strizhevsky <alexxst@gmail.com>,
==> == Midge Shaojun Tan <ShaojunMidge.Tan@audiocodes.com>,
==> == "youtux@gmail.com" <youtux@gmail.com>,
==> == "linux-usb@vger.kernel.org" <linux-usb@vger.kernel.org>,
==> == "netdev@vger.kernel.org" <netdev@vger.kernel.org>
==> ==Subject: Re: Is this 32-bit NCM?
==> ==
==> ==Hi,
==> ==
==> ==The DHCP packets have the maximum size of dwNtbOutMaxSize=16384, while
==> ==the other packets are less than that. However, the DHCP queries are not
==> ==replied in time either, there's always some delay.
==> ==
==> ==By the way, though the device claims to support GET_MAX_DATAGRAM_SIZE,
==> ==but it returns error when the host sends this command to it. I disabled
==> ==this command in NCM driver and tried, but it's the same result.
==> ==
==> ==Regards,
==> ==Kevin
==> ==
==> ==On 12/02/2014 06:02 AM, Eli Britstein wrote:
==> ==> Hi Enrico and all,
==> ==>
==> ==> Maybe I missed something but what is the difference by the driver point of view between the dhcp discover and request (that are ok) to others (like arp, that is nok)?
==> ==> Maybe we can trace to compare them?
==> ==>
==> ==> Sent from my iPhone
==> ==>
==> ==>> On 1 בדצמ 2014, at 23:11, "Enrico Mioso" <mrkiko.rs@gmail.com> wrote:
==> ==>>
==> ==>> So ... I have two ideas left for now.
==> ==>> We know for sure the problem is in the way we TX frames, not the way we RX them
==> ==>> (the way we send, generate them, not the way we receive them).
==> ==>> Si I have two ideas, and I ask for help from the Linux-usb mailing list for
==> ==>> this first one.
==> ==>>
==> ==>> 1 - Does a wayexist to "replay" traffic crom a usb capture?
==> ==>> We might try to take the usb frames generated by Windows, and send them to the
==> ==>> device to see if there is any reaction. It should not be science fiction, I saw
==> ==>> them do that in the eciadsl old project.
==> ==>> 2 - The huawei ndis driver: does it work with these devices?
==> ==>> It should be a little bit out-dated now (at least in terms of dates, it might
==> ==>> work as well): the code is A LOT but, just in case, to see if there is any
==> ==>> chances it'll work. It remains to be seen in which kernels it can actually
==> ==>> compile again.
==> ==>>
==> ==>> If this works we might analyse what's happening and try to debug this out.
==> ==>> But I really would like this to work in the cdc_ncm driver + huawei_cdc_ncm.
==> ==>> Thank you.
==> ==This email and any files transmitted with it are confidential material. They are intended solely for the use of the designated individual or entity to whom they are addressed. If the reader of this message is not the intended recipient, you are hereby notified that any dissemination, use, distribution or copying of this communication is strictly prohibited and may be unlawful.
==> ==
==> ==If you have received this email in error please immediately notify the sender and delete or destroy any copy of this message
==> ==
==This email and any files transmitted with it are confidential material. They are intended solely for the use of the designated individual or entity to whom they are addressed. If the reader of this message is not the intended recipient, you are hereby notified that any dissemination, use, distribution or copying of this communication is strictly prohibited and may be unlawful.
==
==If you have received this email in error please immediately notify the sender and delete or destroy any copy of this message
==
^ permalink raw reply
* Re: Is this 32-bit NCM?
From: Kevin Zhu @ 2014-12-02 6:43 UTC (permalink / raw)
To: Enrico Mioso
Cc: Eli Britstein, Bjørn Mork, Alex Strizhevsky,
Midge Shaojun Tan, youtux@gmail.com, linux-usb@vger.kernel.org,
netdev@vger.kernel.org
In-Reply-To: <alpine.LNX.2.03.1412020729470.1351@gmail.com>
I also tried to define CDC_NCM_DPT_DATAGRAMS_MAX as 1 to eliminate the
paddings, but it did not work either, not even DHCP.
Regards,
Kevin
On 12/02/2014 02:32 PM, Enrico Mioso wrote:
> Hi again Eli,
> Hi Kevin,
> Hi everyone...
>
> As I understand it anyway - the distinction here tends not to be between
> different types of packets.
> It tends to be between received and sent packet.
> We are not able to generate packets that the device retains valid.
> If you manually configure the IP the device would have assigned you via DHCP,
> your system will start answering ARP request, saying that the host with IP
> aa.bb.cc.dd is at aa:bb:cc:dd:ee (using the MC address of the dongle).
> However, the other host (gateway) will never know that. Indeed, it'll continue
> asking who-is at aa.bb.cc.dd ?
> At least, this is the situation I observed in the test system.
>
>
> On Tue, 2 Dec 2014, Kevin Zhu wrote:
>
> ==Date: Tue, 2 Dec 2014 04:53:53
> ==From: Kevin Zhu <Mingying.Zhu@audiocodes.com>
> ==To: Eli Britstein <Eli.Britstein@audiocodes.com>,
> == Enrico Mioso <mrkiko.rs@gmail.com>
> ==Cc: Bjørn Mork <bjorn@mork.no>, Alex Strizhevsky <alexxst@gmail.com>,
> == Midge Shaojun Tan <ShaojunMidge.Tan@audiocodes.com>,
> == "youtux@gmail.com" <youtux@gmail.com>,
> == "linux-usb@vger.kernel.org" <linux-usb@vger.kernel.org>,
> == "netdev@vger.kernel.org" <netdev@vger.kernel.org>
> ==Subject: Re: Is this 32-bit NCM?
> ==
> ==Hi,
> ==
> ==The DHCP packets have the maximum size of dwNtbOutMaxSize=16384, while
> ==the other packets are less than that. However, the DHCP queries are not
> ==replied in time either, there's always some delay.
> ==
> ==By the way, though the device claims to support GET_MAX_DATAGRAM_SIZE,
> ==but it returns error when the host sends this command to it. I disabled
> ==this command in NCM driver and tried, but it's the same result.
> ==
> ==Regards,
> ==Kevin
> ==
> ==On 12/02/2014 06:02 AM, Eli Britstein wrote:
> ==> Hi Enrico and all,
> ==>
> ==> Maybe I missed something but what is the difference by the driver point of view between the dhcp discover and request (that are ok) to others (like arp, that is nok)?
> ==> Maybe we can trace to compare them?
> ==>
> ==> Sent from my iPhone
> ==>
> ==>> On 1 בדצמ 2014, at 23:11, "Enrico Mioso" <mrkiko.rs@gmail.com> wrote:
> ==>>
> ==>> So ... I have two ideas left for now.
> ==>> We know for sure the problem is in the way we TX frames, not the way we RX them
> ==>> (the way we send, generate them, not the way we receive them).
> ==>> Si I have two ideas, and I ask for help from the Linux-usb mailing list for
> ==>> this first one.
> ==>>
> ==>> 1 - Does a wayexist to "replay" traffic crom a usb capture?
> ==>> We might try to take the usb frames generated by Windows, and send them to the
> ==>> device to see if there is any reaction. It should not be science fiction, I saw
> ==>> them do that in the eciadsl old project.
> ==>> 2 - The huawei ndis driver: does it work with these devices?
> ==>> It should be a little bit out-dated now (at least in terms of dates, it might
> ==>> work as well): the code is A LOT but, just in case, to see if there is any
> ==>> chances it'll work. It remains to be seen in which kernels it can actually
> ==>> compile again.
> ==>>
> ==>> If this works we might analyse what's happening and try to debug this out.
> ==>> But I really would like this to work in the cdc_ncm driver + huawei_cdc_ncm.
> ==>> Thank you.
> ==This email and any files transmitted with it are confidential material. They are intended solely for the use of the designated individual or entity to whom they are addressed. If the reader of this message is not the intended recipient, you are hereby notified that any dissemination, use, distribution or copying of this communication is strictly prohibited and may be unlawful.
> ==
> ==If you have received this email in error please immediately notify the sender and delete or destroy any copy of this message
> ==
This email and any files transmitted with it are confidential material. They are intended solely for the use of the designated individual or entity to whom they are addressed. If the reader of this message is not the intended recipient, you are hereby notified that any dissemination, use, distribution or copying of this communication is strictly prohibited and may be unlawful.
If you have received this email in error please immediately notify the sender and delete or destroy any copy of this message
^ permalink raw reply
* Re: Is this 32-bit NCM?
From: Enrico Mioso @ 2014-12-02 6:32 UTC (permalink / raw)
To: Kevin Zhu
Cc: Eli Britstein, Bjørn Mork, Alex Strizhevsky,
Midge Shaojun Tan, youtux-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org,
linux-usb-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <547D37CA.7050506-6C2+4RG2qWF0ubjbjo6WXg@public.gmane.org>
[-- Attachment #1: Type: TEXT/PLAIN, Size: 4009 bytes --]
Hi again Eli,
Hi Kevin,
Hi everyone...
As I understand it anyway - the distinction here tends not to be between
different types of packets.
It tends to be between received and sent packet.
We are not able to generate packets that the device retains valid.
If you manually configure the IP the device would have assigned you via DHCP,
your system will start answering ARP request, saying that the host with IP
aa.bb.cc.dd is at aa:bb:cc:dd:ee (using the MC address of the dongle).
However, the other host (gateway) will never know that. Indeed, it'll continue
asking who-is at aa.bb.cc.dd ?
At least, this is the situation I observed in the test system.
On Tue, 2 Dec 2014, Kevin Zhu wrote:
==Date: Tue, 2 Dec 2014 04:53:53
==From: Kevin Zhu <Mingying.Zhu-6C2+4RG2qWF0ubjbjo6WXg@public.gmane.org>
==To: Eli Britstein <Eli.Britstein-6C2+4RG2qWF0ubjbjo6WXg@public.gmane.org>,
== Enrico Mioso <mrkiko.rs-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
==Cc: Bjørn Mork <bjorn-yOkvZcmFvRU@public.gmane.org>, Alex Strizhevsky <alexxst-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>,
== Midge Shaojun Tan <ShaojunMidge.Tan-6C2+4RG2qWF0ubjbjo6WXg@public.gmane.org>,
== "youtux-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org" <youtux-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>,
== "linux-usb-u79uwXL29TY76Z2rM5mHXA@public.gmane.org" <linux-usb-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>,
== "netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org" <netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>
==Subject: Re: Is this 32-bit NCM?
==
==Hi,
==
==The DHCP packets have the maximum size of dwNtbOutMaxSize=16384, while
==the other packets are less than that. However, the DHCP queries are not
==replied in time either, there's always some delay.
==
==By the way, though the device claims to support GET_MAX_DATAGRAM_SIZE,
==but it returns error when the host sends this command to it. I disabled
==this command in NCM driver and tried, but it's the same result.
==
==Regards,
==Kevin
==
==On 12/02/2014 06:02 AM, Eli Britstein wrote:
==> Hi Enrico and all,
==>
==> Maybe I missed something but what is the difference by the driver point of view between the dhcp discover and request (that are ok) to others (like arp, that is nok)?
==> Maybe we can trace to compare them?
==>
==> Sent from my iPhone
==>
==>> On 1 בדצמ 2014, at 23:11, "Enrico Mioso" <mrkiko.rs-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
==>>
==>> So ... I have two ideas left for now.
==>> We know for sure the problem is in the way we TX frames, not the way we RX them
==>> (the way we send, generate them, not the way we receive them).
==>> Si I have two ideas, and I ask for help from the Linux-usb mailing list for
==>> this first one.
==>>
==>> 1 - Does a wayexist to "replay" traffic crom a usb capture?
==>> We might try to take the usb frames generated by Windows, and send them to the
==>> device to see if there is any reaction. It should not be science fiction, I saw
==>> them do that in the eciadsl old project.
==>> 2 - The huawei ndis driver: does it work with these devices?
==>> It should be a little bit out-dated now (at least in terms of dates, it might
==>> work as well): the code is A LOT but, just in case, to see if there is any
==>> chances it'll work. It remains to be seen in which kernels it can actually
==>> compile again.
==>>
==>> If this works we might analyse what's happening and try to debug this out.
==>> But I really would like this to work in the cdc_ncm driver + huawei_cdc_ncm.
==>> Thank you.
==This email and any files transmitted with it are confidential material. They are intended solely for the use of the designated individual or entity to whom they are addressed. If the reader of this message is not the intended recipient, you are hereby notified that any dissemination, use, distribution or copying of this communication is strictly prohibited and may be unlawful.
==
==If you have received this email in error please immediately notify the sender and delete or destroy any copy of this message
==
^ permalink raw reply
* Re: Is this 32-bit NCM?
From: Enrico Mioso @ 2014-12-02 6:12 UTC (permalink / raw)
To: Eli Britstein
Cc: Kevin Zhu, Bjørn Mork, Alex Strizhevsky, Midge Shaojun Tan,
youtux@gmail.com, linux-usb@vger.kernel.org,
netdev@vger.kernel.org
In-Reply-To: <B315C8AC-3B33-406B-A728-14F8FAAAE986@audiocodes.com>
[-- Attachment #1: Type: TEXT/PLAIN, Size: 3280 bytes --]
And - let's be precise: Eli is right, in some cases (with some dongles /
firmwares, including the one I was / am testing remotely), sometimes DHCP is
answered.
On forums, people experience intermittent results.
The only thing I am thinking now is thet the firmware cares about the content
of the padding area. Crazy, stupid but... I don't know what to think about it
now.
I looked at the .inf file values: and I would suggest to all of you who can run
the installation of the product under Windows, to look at the registry key that
are set and that the drivers are caring about.
In my opinion here should be something interesting.
Looking at the inf files of the *ecm* drivers and tracking down registry values
might be effectively useful.
Enrico.
On Mon, 1 Dec 2014, Eli Britstein wrote:
==Date: Mon, 1 Dec 2014 23:02:16
==From: Eli Britstein <Eli.Britstein@audiocodes.com>
==To: Enrico Mioso <mrkiko.rs@gmail.com>
==Cc: Kevin Zhu <Mingying.Zhu@audiocodes.com>, Bjørn Mork <bjorn@mork.no>,
== Alex Strizhevsky <alexxst@gmail.com>,
== Midge Shaojun Tan <ShaojunMidge.Tan@audiocodes.com>,
== "youtux@gmail.com" <youtux@gmail.com>,
== "linux-usb@vger.kernel.org" <linux-usb@vger.kernel.org>,
== "netdev@vger.kernel.org" <netdev@vger.kernel.org>
==Subject: Re: Is this 32-bit NCM?
==
==Hi Enrico and all,
==
==Maybe I missed something but what is the difference by the driver point of view between the dhcp discover and request (that are ok) to others (like arp, that is nok)?
==Maybe we can trace to compare them?
==
==Sent from my iPhone
==
==> On 1 בדצמ 2014, at 23:11, "Enrico Mioso" <mrkiko.rs@gmail.com> wrote:
==>
==> So ... I have two ideas left for now.
==> We know for sure the problem is in the way we TX frames, not the way we RX them
==> (the way we send, generate them, not the way we receive them).
==> Si I have two ideas, and I ask for help from the Linux-usb mailing list for
==> this first one.
==>
==> 1 - Does a wayexist to "replay" traffic crom a usb capture?
==> We might try to take the usb frames generated by Windows, and send them to the
==> device to see if there is any reaction. It should not be science fiction, I saw
==> them do that in the eciadsl old project.
==> 2 - The huawei ndis driver: does it work with these devices?
==> It should be a little bit out-dated now (at least in terms of dates, it might
==> work as well): the code is A LOT but, just in case, to see if there is any
==> chances it'll work. It remains to be seen in which kernels it can actually
==> compile again.
==>
==> If this works we might analyse what's happening and try to debug this out.
==> But I really would like this to work in the cdc_ncm driver + huawei_cdc_ncm.
==> Thank you.
==
==________________________________
==
==This email and any files transmitted with it are confidential material. They are intended solely for the use of the designated individual or entity to whom they are addressed. If the reader of this message is not the intended recipient, you are hereby notified that any dissemination, use, distribution or copying of this communication is strictly prohibited and may be unlawful.
==
==If you have received this email in error please immediately notify the sender and delete or destroy any copy of this message
==
^ permalink raw reply
* Re: Is this 32-bit NCM?
From: Enrico Mioso @ 2014-12-02 6:07 UTC (permalink / raw)
To: Eli Britstein
Cc: Kevin Zhu, Bjørn Mork, Alex Strizhevsky, Midge Shaojun Tan,
youtux-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org,
linux-usb-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <B315C8AC-3B33-406B-A728-14F8FAAAE986-6C2+4RG2qWF0ubjbjo6WXg@public.gmane.org>
[-- Attachment #1: Type: TEXT/PLAIN, Size: 3404 bytes --]
Hi Eli,
Hi Kevin,
Hi everyone.
Hoping Kevin answered your question - as he did so in a way I could not have
done better.
It seems a matter of the size, even if I am suspecting this could also all
relate to some strange behaviour from the firmware side.
However - I tried also changing the MTU to smaller values, and "shortcircuit"
the negotiation of some NCM parameters, with no results.
Yes - confirming the device returns an error in this case.
Interesting also to note that the device claims to be
"before NCM errata".
See specs.
On Mon, 1 Dec 2014, Eli Britstein wrote:
==Date: Mon, 1 Dec 2014 23:02:16
==From: Eli Britstein <Eli.Britstein-6C2+4RG2qWF0ubjbjo6WXg@public.gmane.org>
==To: Enrico Mioso <mrkiko.rs-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
==Cc: Kevin Zhu <Mingying.Zhu-6C2+4RG2qWF0ubjbjo6WXg@public.gmane.org>, Bjørn Mork <bjorn-yOkvZcmFvRU@public.gmane.org>,
== Alex Strizhevsky <alexxst-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>,
== Midge Shaojun Tan <ShaojunMidge.Tan-6C2+4RG2qWF0ubjbjo6WXg@public.gmane.org>,
== "youtux-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org" <youtux-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>,
== "linux-usb-u79uwXL29TY76Z2rM5mHXA@public.gmane.org" <linux-usb-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>,
== "netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org" <netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>
==Subject: Re: Is this 32-bit NCM?
==
==Hi Enrico and all,
==
==Maybe I missed something but what is the difference by the driver point of view between the dhcp discover and request (that are ok) to others (like arp, that is nok)?
==Maybe we can trace to compare them?
==
==Sent from my iPhone
==
==> On 1 בדצמ 2014, at 23:11, "Enrico Mioso" <mrkiko.rs-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
==>
==> So ... I have two ideas left for now.
==> We know for sure the problem is in the way we TX frames, not the way we RX them
==> (the way we send, generate them, not the way we receive them).
==> Si I have two ideas, and I ask for help from the Linux-usb mailing list for
==> this first one.
==>
==> 1 - Does a wayexist to "replay" traffic crom a usb capture?
==> We might try to take the usb frames generated by Windows, and send them to the
==> device to see if there is any reaction. It should not be science fiction, I saw
==> them do that in the eciadsl old project.
==> 2 - The huawei ndis driver: does it work with these devices?
==> It should be a little bit out-dated now (at least in terms of dates, it might
==> work as well): the code is A LOT but, just in case, to see if there is any
==> chances it'll work. It remains to be seen in which kernels it can actually
==> compile again.
==>
==> If this works we might analyse what's happening and try to debug this out.
==> But I really would like this to work in the cdc_ncm driver + huawei_cdc_ncm.
==> Thank you.
==
==________________________________
==
==This email and any files transmitted with it are confidential material. They are intended solely for the use of the designated individual or entity to whom they are addressed. If the reader of this message is not the intended recipient, you are hereby notified that any dissemination, use, distribution or copying of this communication is strictly prohibited and may be unlawful.
==
==If you have received this email in error please immediately notify the sender and delete or destroy any copy of this message
==
^ permalink raw reply
* [PATCH net-next] rtnetlink: delay RTM_DELLINK notification until after ndo_uninit()
From: Mahesh Bandewar @ 2014-12-02 5:54 UTC (permalink / raw)
To: netdev
Cc: David Miller, Eric Dumazet, Roopa Prabhu, Toshiaki Makita,
Mahesh Bandewar
The commit 56bfa7ee7c ("unregister_netdevice : move RTM_DELLINK to
until after ndo_uninit") tried to do this ealier but while doing so
it created a problem. Unfortunately the delayed rtmsg_ifinfo() also
delayed call to fill_info(). So this translated into asking driver
to remove private state and then quert it's private state. This
could have catastropic consequences.
This change breaks the rtmsg_ifinfo() into two parts - one fills the
skb by calling fill_info() prior to calling ndo_uninit() and the second
part just sends the message using the skb filled earlier.
It was brought to notice when last link is deleted from an ipvlan device
when it has free-ed the port and the subsequent .fill_info() call is
trying to get the info from the port.
kernel: [ 255.139429] ------------[ cut here ]------------
kernel: [ 255.139439] WARNING: CPU: 12 PID: 11173 at net/core/rtnetlink.c:2238 rtmsg_ifinfo+0x100/0x110()
kernel: [ 255.139493] Modules linked in: ipvlan bonding w1_therm ds2482 wire cdc_acm ehci_pci ehci_hcd i2c_dev i2c_i801 i2c_core msr cpuid bnx2x ptp pps_core mdio libcrc32c
kernel: [ 255.139513] CPU: 12 PID: 11173 Comm: ip Not tainted 3.18.0-smp-DEV #167
kernel: [ 255.139514] Hardware name: Intel RML,PCH/Ibis_QC_18, BIOS 1.0.10 05/15/2012
kernel: [ 255.139515] 0000000000000009 ffff880851b6b828 ffffffff815d87f4 00000000000000e0
kernel: [ 255.139516] 0000000000000000 ffff880851b6b868 ffffffff8109c29c 0000000000000000
kernel: [ 255.139518] 00000000ffffffa6 00000000000000d0 ffffffff81aaf580 0000000000000011
kernel: [ 255.139520] Call Trace:
kernel: [ 255.139527] [<ffffffff815d87f4>] dump_stack+0x46/0x58
kernel: [ 255.139531] [<ffffffff8109c29c>] warn_slowpath_common+0x8c/0xc0
kernel: [ 255.139540] [<ffffffff8109c2ea>] warn_slowpath_null+0x1a/0x20
kernel: [ 255.139544] [<ffffffff8150d570>] rtmsg_ifinfo+0x100/0x110
kernel: [ 255.139547] [<ffffffff814f78b5>] rollback_registered_many+0x1d5/0x2d0
kernel: [ 255.139549] [<ffffffff814f79cf>] unregister_netdevice_many+0x1f/0xb0
kernel: [ 255.139551] [<ffffffff8150acab>] rtnl_dellink+0xbb/0x110
kernel: [ 255.139553] [<ffffffff8150da90>] rtnetlink_rcv_msg+0xa0/0x240
kernel: [ 255.139557] [<ffffffff81329283>] ? rhashtable_lookup_compare+0x43/0x80
kernel: [ 255.139558] [<ffffffff8150d9f0>] ? __rtnl_unlock+0x20/0x20
kernel: [ 255.139562] [<ffffffff8152cb11>] netlink_rcv_skb+0xb1/0xc0
kernel: [ 255.139563] [<ffffffff8150a495>] rtnetlink_rcv+0x25/0x40
kernel: [ 255.139565] [<ffffffff8152c398>] netlink_unicast+0x178/0x230
kernel: [ 255.139567] [<ffffffff8152c75f>] netlink_sendmsg+0x30f/0x420
kernel: [ 255.139571] [<ffffffff814e0b0c>] sock_sendmsg+0x9c/0xd0
kernel: [ 255.139575] [<ffffffff811d1d7f>] ? rw_copy_check_uvector+0x6f/0x130
kernel: [ 255.139577] [<ffffffff814e11c9>] ? copy_msghdr_from_user+0x139/0x1b0
kernel: [ 255.139578] [<ffffffff814e1774>] ___sys_sendmsg+0x304/0x310
kernel: [ 255.139581] [<ffffffff81198723>] ? handle_mm_fault+0xca3/0xde0
kernel: [ 255.139585] [<ffffffff811ebc4c>] ? destroy_inode+0x3c/0x70
kernel: [ 255.139589] [<ffffffff8108e6ec>] ? __do_page_fault+0x20c/0x500
kernel: [ 255.139597] [<ffffffff811e8336>] ? dput+0xb6/0x190
kernel: [ 255.139606] [<ffffffff811f05f6>] ? mntput+0x26/0x40
kernel: [ 255.139611] [<ffffffff811d2b94>] ? __fput+0x174/0x1e0
kernel: [ 255.139613] [<ffffffff814e2129>] __sys_sendmsg+0x49/0x90
kernel: [ 255.139615] [<ffffffff814e2182>] SyS_sendmsg+0x12/0x20
kernel: [ 255.139617] [<ffffffff815df092>] system_call_fastpath+0x12/0x17
kernel: [ 255.139619] ---[ end trace 5e6703e87d984f6b ]---
Signed-off-by: Mahesh Bandewar <maheshb@google.com>
Report-by: Toshiaki Makita <makita.toshiaki@lab.ntt.co.jp>
Cc: Eric Dumazet <edumazet@google.com>
Cc: Roopa Prabhu <roopa@cumulusnetworks.com>
Cc: David S. Miller <davem@davemloft.net>
---
drivers/net/bonding/bond_main.c | 4 ++--
include/linux/rtnetlink.h | 6 +++++-
include/net/bonding.h | 8 ++++----
net/core/dev.c | 26 ++++++++++++++++----------
net/core/rtnetlink.c | 20 ++++++++++++++++----
5 files changed, 43 insertions(+), 21 deletions(-)
diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index 184c434ae305..06206a1439a4 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -1135,7 +1135,7 @@ static int bond_master_upper_dev_link(struct net_device *bond_dev,
if (err)
return err;
slave_dev->flags |= IFF_SLAVE;
- rtmsg_ifinfo(RTM_NEWLINK, slave_dev, IFF_SLAVE, GFP_KERNEL);
+ rtmsg_ifinfo(RTM_NEWLINK, slave_dev, IFF_SLAVE, GFP_KERNEL, false);
return 0;
}
@@ -1144,7 +1144,7 @@ static void bond_upper_dev_unlink(struct net_device *bond_dev,
{
netdev_upper_dev_unlink(slave_dev, bond_dev);
slave_dev->flags &= ~IFF_SLAVE;
- rtmsg_ifinfo(RTM_NEWLINK, slave_dev, IFF_SLAVE, GFP_KERNEL);
+ rtmsg_ifinfo(RTM_NEWLINK, slave_dev, IFF_SLAVE, GFP_KERNEL, false);
}
static struct slave *bond_alloc_slave(struct bonding *bond)
diff --git a/include/linux/rtnetlink.h b/include/linux/rtnetlink.h
index 6cacbce1a06c..545dd0b8c83d 100644
--- a/include/linux/rtnetlink.h
+++ b/include/linux/rtnetlink.h
@@ -16,7 +16,11 @@ extern int rtnetlink_put_metrics(struct sk_buff *skb, u32 *metrics);
extern int rtnl_put_cacheinfo(struct sk_buff *skb, struct dst_entry *dst,
u32 id, long expires, u32 error);
-void rtmsg_ifinfo(int type, struct net_device *dev, unsigned change, gfp_t flags);
+struct sk_buff *rtmsg_ifinfo(int type, struct net_device *dev, unsigned change,
+ gfp_t flags, bool fill_only);
+void rtmsg_ifinfo_send(struct sk_buff *skb, struct net_device *dev,
+ gfp_t flags);
+
/* RTNL is used as a global lock for all changes to network configuration */
extern void rtnl_lock(void);
diff --git a/include/net/bonding.h b/include/net/bonding.h
index 983a94b86b95..ea09f6c5af51 100644
--- a/include/net/bonding.h
+++ b/include/net/bonding.h
@@ -315,7 +315,7 @@ static inline void bond_set_active_slave(struct slave *slave)
{
if (slave->backup) {
slave->backup = 0;
- rtmsg_ifinfo(RTM_NEWLINK, slave->dev, 0, GFP_ATOMIC);
+ rtmsg_ifinfo(RTM_NEWLINK, slave->dev, 0, GFP_ATOMIC, false);
}
}
@@ -323,7 +323,7 @@ static inline void bond_set_backup_slave(struct slave *slave)
{
if (!slave->backup) {
slave->backup = 1;
- rtmsg_ifinfo(RTM_NEWLINK, slave->dev, 0, GFP_ATOMIC);
+ rtmsg_ifinfo(RTM_NEWLINK, slave->dev, 0, GFP_ATOMIC, false);
}
}
@@ -335,7 +335,7 @@ static inline void bond_set_slave_state(struct slave *slave,
slave->backup = slave_state;
if (notify) {
- rtmsg_ifinfo(RTM_NEWLINK, slave->dev, 0, GFP_ATOMIC);
+ rtmsg_ifinfo(RTM_NEWLINK, slave->dev, 0, GFP_ATOMIC, false);
slave->should_notify = 0;
} else {
if (slave->should_notify)
@@ -365,7 +365,7 @@ static inline void bond_slave_state_notify(struct bonding *bond)
bond_for_each_slave(bond, tmp, iter) {
if (tmp->should_notify) {
- rtmsg_ifinfo(RTM_NEWLINK, tmp->dev, 0, GFP_ATOMIC);
+ rtmsg_ifinfo(RTM_NEWLINK, tmp->dev, 0, GFP_ATOMIC, false);
tmp->should_notify = 0;
}
}
diff --git a/net/core/dev.c b/net/core/dev.c
index ac4836241a96..29bc78d5e6cb 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -1230,7 +1230,7 @@ void netdev_state_change(struct net_device *dev)
change_info.flags_changed = 0;
call_netdevice_notifiers_info(NETDEV_CHANGE, dev,
&change_info.info);
- rtmsg_ifinfo(RTM_NEWLINK, dev, 0, GFP_KERNEL);
+ rtmsg_ifinfo(RTM_NEWLINK, dev, 0, GFP_KERNEL, false);
}
}
EXPORT_SYMBOL(netdev_state_change);
@@ -1319,7 +1319,7 @@ int dev_open(struct net_device *dev)
if (ret < 0)
return ret;
- rtmsg_ifinfo(RTM_NEWLINK, dev, IFF_UP|IFF_RUNNING, GFP_KERNEL);
+ rtmsg_ifinfo(RTM_NEWLINK, dev, IFF_UP|IFF_RUNNING, GFP_KERNEL, false);
call_netdevice_notifiers(NETDEV_UP, dev);
return ret;
@@ -1396,7 +1396,8 @@ static int dev_close_many(struct list_head *head)
__dev_close_many(head);
list_for_each_entry_safe(dev, tmp, head, close_list) {
- rtmsg_ifinfo(RTM_NEWLINK, dev, IFF_UP|IFF_RUNNING, GFP_KERNEL);
+ rtmsg_ifinfo(RTM_NEWLINK, dev, IFF_UP|IFF_RUNNING,
+ GFP_KERNEL, false);
call_netdevice_notifiers(NETDEV_DOWN, dev);
list_del_init(&dev->close_list);
}
@@ -5683,7 +5684,7 @@ void __dev_notify_flags(struct net_device *dev, unsigned int old_flags,
unsigned int changes = dev->flags ^ old_flags;
if (gchanges)
- rtmsg_ifinfo(RTM_NEWLINK, dev, gchanges, GFP_ATOMIC);
+ rtmsg_ifinfo(RTM_NEWLINK, dev, gchanges, GFP_ATOMIC, false);
if (changes & IFF_UP) {
if (dev->flags & IFF_UP)
@@ -5925,6 +5926,8 @@ static void rollback_registered_many(struct list_head *head)
synchronize_net();
list_for_each_entry(dev, head, unreg_list) {
+ struct sk_buff *skb = NULL;
+
/* Shutdown queueing discipline. */
dev_shutdown(dev);
@@ -5934,6 +5937,10 @@ static void rollback_registered_many(struct list_head *head)
*/
call_netdevice_notifiers(NETDEV_UNREGISTER, dev);
+ if (!dev->rtnl_link_ops ||
+ dev->rtnl_link_state == RTNL_LINK_INITIALIZED)
+ skb = rtmsg_ifinfo(RTM_DELLINK, dev, ~0U, GFP_KERNEL, true);
+
/*
* Flush the unicast and multicast chains
*/
@@ -5943,9 +5950,8 @@ static void rollback_registered_many(struct list_head *head)
if (dev->netdev_ops->ndo_uninit)
dev->netdev_ops->ndo_uninit(dev);
- if (!dev->rtnl_link_ops ||
- dev->rtnl_link_state == RTNL_LINK_INITIALIZED)
- rtmsg_ifinfo(RTM_DELLINK, dev, ~0U, GFP_KERNEL);
+ if (skb)
+ rtmsg_ifinfo_send(skb, dev, GFP_KERNEL);
/* Notifier chain MUST detach us all upper devices. */
WARN_ON(netdev_has_any_upper_dev(dev));
@@ -6334,7 +6340,7 @@ int register_netdevice(struct net_device *dev)
*/
if (!dev->rtnl_link_ops ||
dev->rtnl_link_state == RTNL_LINK_INITIALIZED)
- rtmsg_ifinfo(RTM_NEWLINK, dev, ~0U, GFP_KERNEL);
+ rtmsg_ifinfo(RTM_NEWLINK, dev, ~0U, GFP_KERNEL, false);
out:
return ret;
@@ -6959,7 +6965,7 @@ int dev_change_net_namespace(struct net_device *dev, struct net *net, const char
call_netdevice_notifiers(NETDEV_UNREGISTER, dev);
rcu_barrier();
call_netdevice_notifiers(NETDEV_UNREGISTER_FINAL, dev);
- rtmsg_ifinfo(RTM_DELLINK, dev, ~0U, GFP_KERNEL);
+ rtmsg_ifinfo(RTM_DELLINK, dev, ~0U, GFP_KERNEL, false);
/*
* Flush the unicast and multicast chains
@@ -7000,7 +7006,7 @@ int dev_change_net_namespace(struct net_device *dev, struct net *net, const char
* Prevent userspace races by waiting until the network
* device is fully setup before sending notifications.
*/
- rtmsg_ifinfo(RTM_NEWLINK, dev, ~0U, GFP_KERNEL);
+ rtmsg_ifinfo(RTM_NEWLINK, dev, ~0U, GFP_KERNEL, false);
synchronize_net();
err = 0;
diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
index b9b7dfaf202b..1035d8cdbc08 100644
--- a/net/core/rtnetlink.c
+++ b/net/core/rtnetlink.c
@@ -2220,8 +2220,16 @@ static int rtnl_dump_all(struct sk_buff *skb, struct netlink_callback *cb)
return skb->len;
}
-void rtmsg_ifinfo(int type, struct net_device *dev, unsigned int change,
- gfp_t flags)
+void rtmsg_ifinfo_send(struct sk_buff *skb, struct net_device *dev, gfp_t flags)
+{
+ struct net *net = dev_net(dev);
+
+ rtnl_notify(skb, net, 0, RTNLGRP_LINK, NULL, flags);
+}
+EXPORT_SYMBOL(rtmsg_ifinfo_send);
+
+struct sk_buff *rtmsg_ifinfo(int type, struct net_device *dev,
+ unsigned int change, gfp_t flags, bool fill_only)
{
struct net *net = dev_net(dev);
struct sk_buff *skb;
@@ -2239,11 +2247,15 @@ void rtmsg_ifinfo(int type, struct net_device *dev, unsigned int change,
kfree_skb(skb);
goto errout;
}
+ if (fill_only)
+ return skb;
+
rtnl_notify(skb, net, 0, RTNLGRP_LINK, NULL, flags);
- return;
+ return NULL;
errout:
if (err < 0)
rtnl_set_sk_err(net, RTNLGRP_LINK, err);
+ return NULL;
}
EXPORT_SYMBOL(rtmsg_ifinfo);
@@ -3011,7 +3023,7 @@ static int rtnetlink_event(struct notifier_block *this, unsigned long event, voi
case NETDEV_JOIN:
break;
default:
- rtmsg_ifinfo(RTM_NEWLINK, dev, 0, GFP_KERNEL);
+ rtmsg_ifinfo(RTM_NEWLINK, dev, 0, GFP_KERNEL, false);
break;
}
return NOTIFY_DONE;
--
2.2.0.rc0.207.ga3a616c
^ permalink raw reply related
* Re: [PATCH V2 net-next] udp: Neaten and reduce size of compute_score functions
From: Joe Perches @ 2014-12-02 5:26 UTC (permalink / raw)
To: Eric Dumazet; +Cc: netdev, LKML
In-Reply-To: <1417496925.5303.18.camel@edumazet-glaptop2.roam.corp.google.com>
On Mon, 2014-12-01 at 21:08 -0800, Eric Dumazet wrote:
> On Mon, 2014-12-01 at 20:29 -0800, Joe Perches wrote:
> > The compute_score functions are a bit difficult to read.
> >
> > Neaten them a bit to reduce object sizes and make them a
> > bit more intelligible.
> >
> > Return early to avoid indentation and avoid unnecessary
> > initializations.
> >
> > (allyesconfig, but w/ -O2 and no profiling)
>
> hmm... Not sure how you get such large numbers...
Nor I particularly, but I do with gcc 4.9.1
> > $ size net/ipv[46]/udp.o.*
> > text data bss dec hex filename
> > 28680 1184 25 29889 74c1 net/ipv4/udp.o.new
> > 28756 1184 25 29965 750d net/ipv4/udp.o.old
[]
> Here I have :
>
> # size net/ipv4/udp.o.*
> text data bss dec hex filename
> 21989 616 9 22614 5856 net/ipv4/udp.o.old
> 21957 616 9 22582 5836 net/ipv4/udp.o.new
Curious. What gcc version?
with that 4.9.1 gcc version and a defconfig (x86-64) I get:
$ size net/ipv[46]/udp.o*
text data bss dec hex filename
21328 672 9 22009 55f9 net/ipv4/udp.o.new
21312 672 9 21993 55e9 net/ipv4/udp.o.old
14463 588 2 15053 3acd net/ipv6/udp.o.new
14527 588 2 15117 3b0d net/ipv6/udp.o.old
and defconfig x86-32:
$ size net/ipv[46]/udp.o*
text data bss dec hex filename
19626 324 5 19955 4df3 net/ipv4/udp.o.new
19706 324 5 20035 4e43 net/ipv4/udp.o.old
14189 300 2 14491 389b net/ipv6/udp.o.new
14125 300 2 14427 385b net/ipv6/udp.o.old
> With CONFIG_CC_OPTIMIZE_FOR_SIZE=y I even have an opposite result (code
> gets bigger after your patch)
>
> # size net/ipv4/udp.o.*
> text data bss dec hex filename
> 17242 600 9 17851 45bb net/ipv4/udp.o.old
> 17256 600 9 17865 45c9 net/ipv4/udp.o.new
>
> Anyway, your patch looks fine to me, no matter what the code size is.
>
> Acked-by: Eric Dumazet <edumazet@google.com>
^ permalink raw reply
* Re: [PATCH V2 net-next] udp: Neaten and reduce size of compute_score functions
From: Eric Dumazet @ 2014-12-02 5:08 UTC (permalink / raw)
To: Joe Perches; +Cc: netdev, LKML
In-Reply-To: <1417494546.4894.12.camel@perches.com>
On Mon, 2014-12-01 at 20:29 -0800, Joe Perches wrote:
> The compute_score functions are a bit difficult to read.
>
> Neaten them a bit to reduce object sizes and make them a
> bit more intelligible.
>
> Return early to avoid indentation and avoid unnecessary
> initializations.
>
> (allyesconfig, but w/ -O2 and no profiling)
hmm... Not sure how you get such large numbers...
>
> $ size net/ipv[46]/udp.o.*
> text data bss dec hex filename
> 28680 1184 25 29889 74c1 net/ipv4/udp.o.new
> 28756 1184 25 29965 750d net/ipv4/udp.o.old
> 17600 1010 2 18612 48b4 net/ipv6/udp.o.new
> 17632 1010 2 18644 48d4 net/ipv6/udp.o.old
Here I have :
# size net/ipv4/udp.o.*
text data bss dec hex filename
21989 616 9 22614 5856 net/ipv4/udp.o.old
21957 616 9 22582 5836 net/ipv4/udp.o.new
With CONFIG_CC_OPTIMIZE_FOR_SIZE=y I even have an opposite result (code
gets bigger after your patch)
# size net/ipv4/udp.o.*
text data bss dec hex filename
17242 600 9 17851 45bb net/ipv4/udp.o.old
17256 600 9 17865 45c9 net/ipv4/udp.o.new
Anyway, your patch looks fine to me, no matter what the code size is.
Acked-by: Eric Dumazet <edumazet@google.com>
^ permalink raw reply
* Re: [PATCH net-next] udp: Neaten and reduce size of compute_score functions
From: Eric Dumazet @ 2014-12-02 4:44 UTC (permalink / raw)
To: Joe Perches; +Cc: netdev, LKML
In-Reply-To: <1417489760.4894.8.camel@perches.com>
On Mon, 2014-12-01 at 19:09 -0800, Joe Perches wrote:
> On Mon, 2014-12-01 at 18:59 -0800, Eric Dumazet wrote:
> > On Mon, 2014-12-01 at 17:39 -0800, Joe Perches wrote:
> > > The compute_score functions are a bit difficult to read.
> > >
> > > Neaten them a bit to reduce object sizes and make them a
> > > bit more intelligible.
> > >
> > > Return early to avoid indentation and avoid unnecessary
> > > initializations.
> []
> > > + if (!(net_eq(sock_net(sk), net) &&
> > > + udp_sk(sk)->udp_port_hash == hnum &&
> > > + !ipv6_only_sock(sk)))
> > > + return -1
> >
> > Or even better :
> >
> >
> > if (!net_eq(sock_net(sk), net) ||
> > udp_sk(sk)->udp_port_hash != hnum ||
> > ipv6_only_sock(sk))
> > return -1;
>
> Hi Eric.
>
> Yeah, I thought about it but thought it
> simpler to not change the logic.
>
> Either way is fine with me.
Your patch does not change the logic at all.
My suggestion is about avoiding double negates which are really hard to
read. This is simple boolean logic rules.
Code was :
if (a && x == y && !ipv6_only_sock(sk))) {
EXPR1;
} else {
return -1;
}
So the 'logical' way of negating the condition is actually to not add
double negations and use :
if (!a || x != y || ipv6_only_sock(sk)))
return -1;
EXPR1;
Instead of the less readable form :
if (!(a && x == y && !ipv6_only_sock(sk))))
return -1;
EXPR1;
You do not have to ask David permission for this, really.
^ permalink raw reply
* Re: [PATCH] Documentation: bindings: net: DPAA corenet binding document
From: Scott Wood @ 2014-12-02 4:39 UTC (permalink / raw)
To: madalin.bucur
Cc: devicetree, linuxppc-dev, netdev, Emilian.Medve, Igal.Liberman
In-Reply-To: <1417169426-11823-1-git-send-email-madalin.bucur@freescale.com>
On Fri, 2014-11-28 at 12:10 +0200, Madalin Bucur wrote:
> Add the device tree binding document for the DPAA corenet node
> and DPAA Ethernet nodes.
>
> Signed-off-by: Madalin Bucur <madalin.bucur@freescale.com>
> ---
> Documentation/devicetree/bindings/net/fsl-dpaa.txt | 31 ++++++++++++++++++++++
> 1 file changed, 31 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/net/fsl-dpaa.txt
>
> diff --git a/Documentation/devicetree/bindings/net/fsl-dpaa.txt b/Documentation/devicetree/bindings/net/fsl-dpaa.txt
> new file mode 100644
> index 0000000..822c668
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/fsl-dpaa.txt
> @@ -0,0 +1,31 @@
> +*DPAA corenet
> +
> +The corenet bus containing all DPAA Ethernet nodes.
What does this have to do with corenet?
> +Required property
> + - compatible: string property. Must include "fsl,dpaa". Can include
> + also "fsl,<SoC>-dpaa".
No need for the <SoC> part. As we previously discussed, the only
purpose of this node is backwards compatibility with the U-Boot MAC
address fixup -- if U-Boot doesn't look for the <SoC> version, then
don't complicate things.
Though, I can't find where U-Boot references this node. Are you sure
it's not using the ethernet%d aliases like everything else, in which
case why do we need this node at all?
-Scott
^ permalink raw reply
* [PATCH V2 net-next] udp: Neaten and reduce size of compute_score functions
From: Joe Perches @ 2014-12-02 4:29 UTC (permalink / raw)
To: Eric Dumazet; +Cc: netdev, LKML
In-Reply-To: <1417489760.4894.8.camel@perches.com>
The compute_score functions are a bit difficult to read.
Neaten them a bit to reduce object sizes and make them a
bit more intelligible.
Return early to avoid indentation and avoid unnecessary
initializations.
(allyesconfig, but w/ -O2 and no profiling)
$ size net/ipv[46]/udp.o.*
text data bss dec hex filename
28680 1184 25 29889 74c1 net/ipv4/udp.o.new
28756 1184 25 29965 750d net/ipv4/udp.o.old
17600 1010 2 18612 48b4 net/ipv6/udp.o.new
17632 1010 2 18644 48d4 net/ipv6/udp.o.old
Signed-off-by: Joe Perches <joe@perches.com>
---
Change the && == blocks to || !=
No change in compiled object files.
Keeps Eric happy too.
net/ipv4/udp.c | 111 +++++++++++++++++++++++++++++++-------------------------
net/ipv6/udp.c | 113 ++++++++++++++++++++++++++++++++-------------------------
2 files changed, 125 insertions(+), 99 deletions(-)
diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c
index b2d6068..dd8e006 100644
--- a/net/ipv4/udp.c
+++ b/net/ipv4/udp.c
@@ -336,38 +336,45 @@ int udp_v4_get_port(struct sock *sk, unsigned short snum)
return udp_lib_get_port(sk, snum, ipv4_rcv_saddr_equal, hash2_nulladdr);
}
-static inline int compute_score(struct sock *sk, struct net *net, __be32 saddr,
- unsigned short hnum,
- __be16 sport, __be32 daddr, __be16 dport, int dif)
+static inline int compute_score(struct sock *sk, struct net *net,
+ __be32 saddr, unsigned short hnum, __be16 sport,
+ __be32 daddr, __be16 dport, int dif)
{
- int score = -1;
+ int score;
+ struct inet_sock *inet;
- if (net_eq(sock_net(sk), net) && udp_sk(sk)->udp_port_hash == hnum &&
- !ipv6_only_sock(sk)) {
- struct inet_sock *inet = inet_sk(sk);
+ if (!net_eq(sock_net(sk), net) ||
+ udp_sk(sk)->udp_port_hash != hnum ||
+ ipv6_only_sock(sk))
+ return -1;
- score = (sk->sk_family == PF_INET ? 2 : 1);
- if (inet->inet_rcv_saddr) {
- if (inet->inet_rcv_saddr != daddr)
- return -1;
- score += 4;
- }
- if (inet->inet_daddr) {
- if (inet->inet_daddr != saddr)
- return -1;
- score += 4;
- }
- if (inet->inet_dport) {
- if (inet->inet_dport != sport)
- return -1;
- score += 4;
- }
- if (sk->sk_bound_dev_if) {
- if (sk->sk_bound_dev_if != dif)
- return -1;
- score += 4;
- }
+ score = (sk->sk_family == PF_INET) ? 2 : 1;
+ inet = inet_sk(sk);
+
+ if (inet->inet_rcv_saddr) {
+ if (inet->inet_rcv_saddr != daddr)
+ return -1;
+ score += 4;
+ }
+
+ if (inet->inet_daddr) {
+ if (inet->inet_daddr != saddr)
+ return -1;
+ score += 4;
}
+
+ if (inet->inet_dport) {
+ if (inet->inet_dport != sport)
+ return -1;
+ score += 4;
+ }
+
+ if (sk->sk_bound_dev_if) {
+ if (sk->sk_bound_dev_if != dif)
+ return -1;
+ score += 4;
+ }
+
return score;
}
@@ -378,33 +385,39 @@ static inline int compute_score2(struct sock *sk, struct net *net,
__be32 saddr, __be16 sport,
__be32 daddr, unsigned int hnum, int dif)
{
- int score = -1;
+ int score;
+ struct inet_sock *inet;
+
+ if (!net_eq(sock_net(sk), net) ||
+ ipv6_only_sock(sk))
+ return -1;
- if (net_eq(sock_net(sk), net) && !ipv6_only_sock(sk)) {
- struct inet_sock *inet = inet_sk(sk);
+ inet = inet_sk(sk);
- if (inet->inet_rcv_saddr != daddr)
+ if (inet->inet_rcv_saddr != daddr ||
+ inet->inet_num != hnum)
+ return -1;
+
+ score = (sk->sk_family == PF_INET) ? 2 : 1;
+
+ if (inet->inet_daddr) {
+ if (inet->inet_daddr != saddr)
return -1;
- if (inet->inet_num != hnum)
+ score += 4;
+ }
+
+ if (inet->inet_dport) {
+ if (inet->inet_dport != sport)
return -1;
+ score += 4;
+ }
- score = (sk->sk_family == PF_INET ? 2 : 1);
- if (inet->inet_daddr) {
- if (inet->inet_daddr != saddr)
- return -1;
- score += 4;
- }
- if (inet->inet_dport) {
- if (inet->inet_dport != sport)
- return -1;
- score += 4;
- }
- if (sk->sk_bound_dev_if) {
- if (sk->sk_bound_dev_if != dif)
- return -1;
- score += 4;
- }
+ if (sk->sk_bound_dev_if) {
+ if (sk->sk_bound_dev_if != dif)
+ return -1;
+ score += 4;
}
+
return score;
}
diff --git a/net/ipv6/udp.c b/net/ipv6/udp.c
index 7cfb5d7..7f964322 100644
--- a/net/ipv6/udp.c
+++ b/net/ipv6/udp.c
@@ -148,72 +148,85 @@ static inline int compute_score(struct sock *sk, struct net *net,
const struct in6_addr *daddr, __be16 dport,
int dif)
{
- int score = -1;
+ int score;
+ struct inet_sock *inet;
- if (net_eq(sock_net(sk), net) && udp_sk(sk)->udp_port_hash == hnum &&
- sk->sk_family == PF_INET6) {
- struct inet_sock *inet = inet_sk(sk);
+ if (!net_eq(sock_net(sk), net) ||
+ udp_sk(sk)->udp_port_hash != hnum ||
+ sk->sk_family != PF_INET6)
+ return -1;
- score = 0;
- if (inet->inet_dport) {
- if (inet->inet_dport != sport)
- return -1;
- score++;
- }
- if (!ipv6_addr_any(&sk->sk_v6_rcv_saddr)) {
- if (!ipv6_addr_equal(&sk->sk_v6_rcv_saddr, daddr))
- return -1;
- score++;
- }
- if (!ipv6_addr_any(&sk->sk_v6_daddr)) {
- if (!ipv6_addr_equal(&sk->sk_v6_daddr, saddr))
- return -1;
- score++;
- }
- if (sk->sk_bound_dev_if) {
- if (sk->sk_bound_dev_if != dif)
- return -1;
- score++;
- }
+ score = 0;
+ inet = inet_sk(sk);
+
+ if (inet->inet_dport) {
+ if (inet->inet_dport != sport)
+ return -1;
+ score++;
}
+
+ if (!ipv6_addr_any(&sk->sk_v6_rcv_saddr)) {
+ if (!ipv6_addr_equal(&sk->sk_v6_rcv_saddr, daddr))
+ return -1;
+ score++;
+ }
+
+ if (!ipv6_addr_any(&sk->sk_v6_daddr)) {
+ if (!ipv6_addr_equal(&sk->sk_v6_daddr, saddr))
+ return -1;
+ score++;
+ }
+
+ if (sk->sk_bound_dev_if) {
+ if (sk->sk_bound_dev_if != dif)
+ return -1;
+ score++;
+ }
+
return score;
}
#define SCORE2_MAX (1 + 1 + 1)
static inline int compute_score2(struct sock *sk, struct net *net,
- const struct in6_addr *saddr, __be16 sport,
- const struct in6_addr *daddr, unsigned short hnum,
- int dif)
+ const struct in6_addr *saddr, __be16 sport,
+ const struct in6_addr *daddr,
+ unsigned short hnum, int dif)
{
- int score = -1;
+ int score;
+ struct inet_sock *inet;
- if (net_eq(sock_net(sk), net) && udp_sk(sk)->udp_port_hash == hnum &&
- sk->sk_family == PF_INET6) {
- struct inet_sock *inet = inet_sk(sk);
+ if (!net_eq(sock_net(sk), net) ||
+ udp_sk(sk)->udp_port_hash != hnum ||
+ sk->sk_family != PF_INET6)
+ return -1;
- if (!ipv6_addr_equal(&sk->sk_v6_rcv_saddr, daddr))
+ if (!ipv6_addr_equal(&sk->sk_v6_rcv_saddr, daddr))
+ return -1;
+
+ score = 0;
+ inet = inet_sk(sk);
+
+ if (inet->inet_dport) {
+ if (inet->inet_dport != sport)
return -1;
- score = 0;
- if (inet->inet_dport) {
- if (inet->inet_dport != sport)
- return -1;
- score++;
- }
- if (!ipv6_addr_any(&sk->sk_v6_daddr)) {
- if (!ipv6_addr_equal(&sk->sk_v6_daddr, saddr))
- return -1;
- score++;
- }
- if (sk->sk_bound_dev_if) {
- if (sk->sk_bound_dev_if != dif)
- return -1;
- score++;
- }
+ score++;
}
+
+ if (!ipv6_addr_any(&sk->sk_v6_daddr)) {
+ if (!ipv6_addr_equal(&sk->sk_v6_daddr, saddr))
+ return -1;
+ score++;
+ }
+
+ if (sk->sk_bound_dev_if) {
+ if (sk->sk_bound_dev_if != dif)
+ return -1;
+ score++;
+ }
+
return score;
}
-
/* called with read_rcu_lock() */
static struct sock *udp6_lib_lookup2(struct net *net,
const struct in6_addr *saddr, __be16 sport,
^ permalink raw reply related
* Re: Is this 32-bit NCM?
From: Kevin Zhu @ 2014-12-02 3:53 UTC (permalink / raw)
To: Eli Britstein, Enrico Mioso
Cc: Bjørn Mork, Alex Strizhevsky, Midge Shaojun Tan,
youtux@gmail.com, linux-usb@vger.kernel.org,
netdev@vger.kernel.org
In-Reply-To: <B315C8AC-3B33-406B-A728-14F8FAAAE986@audiocodes.com>
Hi,
The DHCP packets have the maximum size of dwNtbOutMaxSize=16384, while
the other packets are less than that. However, the DHCP queries are not
replied in time either, there's always some delay.
By the way, though the device claims to support GET_MAX_DATAGRAM_SIZE,
but it returns error when the host sends this command to it. I disabled
this command in NCM driver and tried, but it's the same result.
Regards,
Kevin
On 12/02/2014 06:02 AM, Eli Britstein wrote:
> Hi Enrico and all,
>
> Maybe I missed something but what is the difference by the driver point of view between the dhcp discover and request (that are ok) to others (like arp, that is nok)?
> Maybe we can trace to compare them?
>
> Sent from my iPhone
>
>> On 1 בדצמ 2014, at 23:11, "Enrico Mioso" <mrkiko.rs@gmail.com> wrote:
>>
>> So ... I have two ideas left for now.
>> We know for sure the problem is in the way we TX frames, not the way we RX them
>> (the way we send, generate them, not the way we receive them).
>> Si I have two ideas, and I ask for help from the Linux-usb mailing list for
>> this first one.
>>
>> 1 - Does a wayexist to "replay" traffic crom a usb capture?
>> We might try to take the usb frames generated by Windows, and send them to the
>> device to see if there is any reaction. It should not be science fiction, I saw
>> them do that in the eciadsl old project.
>> 2 - The huawei ndis driver: does it work with these devices?
>> It should be a little bit out-dated now (at least in terms of dates, it might
>> work as well): the code is A LOT but, just in case, to see if there is any
>> chances it'll work. It remains to be seen in which kernels it can actually
>> compile again.
>>
>> If this works we might analyse what's happening and try to debug this out.
>> But I really would like this to work in the cdc_ncm driver + huawei_cdc_ncm.
>> Thank you.
This email and any files transmitted with it are confidential material. They are intended solely for the use of the designated individual or entity to whom they are addressed. If the reader of this message is not the intended recipient, you are hereby notified that any dissemination, use, distribution or copying of this communication is strictly prohibited and may be unlawful.
If you have received this email in error please immediately notify the sender and delete or destroy any copy of this message
^ permalink raw reply
* Re: [PATCH RFC v3 0/3] virtio_net: enabling tx interrupts
From: Jason Wang @ 2014-12-02 3:36 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: linux-kernel, netdev
In-Reply-To: <20141201104808.GA16661@redhat.com>
On Mon, Dec 1, 2014 at 6:48 PM, Michael S. Tsirkin <mst@redhat.com>
wrote:
> On Mon, Dec 01, 2014 at 06:14:36PM +0800, Jason Wang wrote:
>>
>>
>> On 10/20/2014 02:52 PM, Michael S. Tsirkin wrote:
>> >RFC patches to enable tx interrupts.
>> >This is to demonstrate how this can be done without
>> >core virtio changes, and to make sure I understand
>> >the new APIs correctly.
>> >
>> >Testing TBD, I was asked for a version for early testing.
>> >
>> >Applies on top of patch: "virtio_net: fix use after free"
>> >that I recently sent.
>> >
>> >Changes from v3:
>> > clean up code, address issues raised by Jason
>> >Changes from v1:
>> > address comments by Jason Wang, use delayed cb everywhere
>> > rebased Jason's patch on top of mine and include it (with
>> some tweaks)
>> >
>> >Jason Wang (1):
>> > virtio-net: optimize free_old_xmit_skbs stats
>> >
>> >Michael S. Tsirkin (2):
>> > virtio_net: enable tx interrupt
>> > virtio_net: bql
>> >
>> > drivers/net/virtio_net.c | 144
>> +++++++++++++++++++++++++++++++++--------------
>> > 1 file changed, 101 insertions(+), 43 deletions(-)
>> >
>>
>> I've run a full tests on this series and see huge regression when
>> zerocopy
>> is disabled. Looks like the reason is zerocopy could coalescing tx
>> completion which greatly reduce the number of tx interrupts.
>
> I think you refer to this code:
>
> /*
> * Trigger polling thread if guest stopped submitting new
> * buffers:
> * in this case, the refcount after decrement will eventually
> * reach 1.
> * We also trigger polling periodically after each 16 packets
> * (the value 16 here is more or less arbitrary, it's tuned to
> * trigger
> * less than 10% of times).
> */
> if (cnt <= 1 || !(cnt % 16))
> vhost_poll_queue(&vq->poll);
>
> ?
> This seems unrelated to interrupt coalescing.
Well, this in fact tries to coalesce 16 packets per tx irq.
More important, zerocopy depends on host nics tx completion.
This means, if host nic coalesces several packets per irq,
zerocopy will probably also do this. vhost_zerocopy_signal_used()
will try to coalesce the tx intrs.
>
> We can easily enable something similar for all tx
> packets, without need for guest configuration.
We can, but we lose the an interface for user to tune for
their applications.
>
> If it's not clear how to do this, let me know, I'll try to put out a
> patch like this in a couple of days.
We can just do this through harding coding the tx-frames to 16
through interrupt coalescing. I don't see obvious differences.
And in my test of RFCv4, I just use tx-frames 16 to get the result.
Without a timer for coalescing, I suspect how much this can help
for e.g 1 session of TCP_RR. It just has at most 1 packet pending
during the test. So in fact no tx completion could be coalesced
in this case.
>
>
>> I will post RFC V4 shortly with interrupt coalescing support. In
>> this
>> version I remove the tx packet cleanup in ndo_start_xmit() since it
>> may
>> reduce the effects of interrupt coalescing.
>
> Maybe split this in a separate patch?
I can, but since just two minor changes compared to V3. Maybe just
document the differences is ok for you?
^ 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