Netdev List
 help / color / mirror / Atom feed
* [PATCH] rtlwifi: rtl8192de: remove pointless conditional before kfree_skb()
From: Wei Yongjun @ 2012-08-28 13:11 UTC (permalink / raw)
  To: Larry.Finger, chaoming_li, linville; +Cc: yongjun_wei, linux-wireless, netdev

From: Wei Yongjun <yongjun_wei@trendmicro.com.cn>

Remove pointless conditional before kfree_skb().

Signed-off-by: Wei Yongjun <yongjun_wei@trendmicro.com.cn>
---
 drivers/net/wireless/rtlwifi/rtl8192de/fw.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/net/wireless/rtlwifi/rtl8192de/fw.c b/drivers/net/wireless/rtlwifi/rtl8192de/fw.c
index 895ae6c..a3aede0 100644
--- a/drivers/net/wireless/rtlwifi/rtl8192de/fw.c
+++ b/drivers/net/wireless/rtlwifi/rtl8192de/fw.c
@@ -570,8 +570,7 @@ static bool _rtl92d_cmd_send_packet(struct ieee80211_hw *hw,
 
 	ring = &rtlpci->tx_ring[BEACON_QUEUE];
 	pskb = __skb_dequeue(&ring->queue);
-	if (pskb)
-		kfree_skb(pskb);
+	kfree_skb(pskb);
 	spin_lock_irqsave(&rtlpriv->locks.irq_th_lock, flags);
 	pdesc = &ring->desc[idx];
 	/* discard output from call below */

^ permalink raw reply related

* [PATCH] skbuff: remove pointless conditional before kfree_skb()
From: Wei Yongjun @ 2012-08-28 13:10 UTC (permalink / raw)
  To: davem; +Cc: yongjun_wei, netdev

From: Wei Yongjun <yongjun_wei@trendmicro.com.cn>

Remove pointless conditional before kfree_skb().

Signed-off-by: Wei Yongjun <yongjun_wei@trendmicro.com.cn>
---
 include/linux/skbuff.h | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index 7632c87..0b846d9 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -2464,8 +2464,7 @@ static inline void nf_conntrack_get_reasm(struct sk_buff *skb)
 }
 static inline void nf_conntrack_put_reasm(struct sk_buff *skb)
 {
-	if (skb)
-		kfree_skb(skb);
+	kfree_skb(skb);
 }
 #endif
 #ifdef CONFIG_BRIDGE_NETFILTER

^ permalink raw reply related

* [PATCH] batman-adv: remove pointless conditional before kfree_skb()
From: Wei Yongjun @ 2012-08-28 13:06 UTC (permalink / raw)
  To: lindner_marek, siwu, ordex, davem; +Cc: yongjun_wei, b.a.t.m.a.n, netdev

From: Wei Yongjun <yongjun_wei@trendmicro.com.cn>

Remove pointless conditional before kfree_skb().

Signed-off-by: Wei Yongjun <yongjun_wei@trendmicro.com.cn>
---
 net/batman-adv/send.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/net/batman-adv/send.c b/net/batman-adv/send.c
index 3b4b2da..534dc46 100644
--- a/net/batman-adv/send.c
+++ b/net/batman-adv/send.c
@@ -99,8 +99,7 @@ void batadv_schedule_bat_ogm(struct batadv_hard_iface *hard_iface)
 
 static void batadv_forw_packet_free(struct batadv_forw_packet *forw_packet)
 {
-	if (forw_packet->skb)
-		kfree_skb(forw_packet->skb);
+	kfree_skb(forw_packet->skb);
 	if (forw_packet->if_incoming)
 		batadv_hardif_free_ref(forw_packet->if_incoming);
 	kfree(forw_packet);

^ permalink raw reply related

* [PATCH] tipc: remove pointless conditional before kfree_skb()
From: Wei Yongjun @ 2012-08-28 13:05 UTC (permalink / raw)
  To: jon.maloy, allan.stephens, davem; +Cc: yongjun_wei, netdev, tipc-discussion

From: Wei Yongjun <yongjun_wei@trendmicro.com.cn>

Remove pointless conditional before kfree_skb().

Signed-off-by: Wei Yongjun <yongjun_wei@trendmicro.com.cn>
---
 net/tipc/port.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/net/tipc/port.c b/net/tipc/port.c
index 07c42fb..9227d2c 100644
--- a/net/tipc/port.c
+++ b/net/tipc/port.c
@@ -770,8 +770,7 @@ static void port_dispatcher_sigh(void *dummy)
 				break;
 			}
 		}
-		if (buf)
-			kfree_skb(buf);
+		kfree_skb(buf);
 		buf = next;
 		continue;
 err:
@@ -819,8 +818,7 @@ err:
 				break;
 			}
 		}
-		if (buf)
-			kfree_skb(buf);
+		kfree_skb(buf);
 		buf = next;
 		continue;
 reject:

^ permalink raw reply related

* [PATCH] cxgb3: remove pointless conditional before kfree_skb()
From: Wei Yongjun @ 2012-08-28 13:04 UTC (permalink / raw)
  To: divy; +Cc: yongjun_wei, netdev

From: Wei Yongjun <yongjun_wei@trendmicro.com.cn>

Remove pointless conditional before kfree_skb().

Signed-off-by: Wei Yongjun <yongjun_wei@trendmicro.com.cn>
---
 drivers/net/ethernet/chelsio/cxgb3/cxgb3_offload.c | 3 +--
 drivers/net/ethernet/chelsio/cxgb3/cxgb3_main.c    | 3 +--
 2 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/chelsio/cxgb3/cxgb3_offload.c b/drivers/net/ethernet/chelsio/cxgb3/cxgb3_offload.c
index 2dbbcbb..aac26f2 100644
--- a/drivers/net/ethernet/chelsio/cxgb3/cxgb3_offload.c
+++ b/drivers/net/ethernet/chelsio/cxgb3/cxgb3_offload.c
@@ -1341,8 +1341,7 @@ void cxgb3_offload_deactivate(struct adapter *adapter)
 	rcu_read_unlock();
 	RCU_INIT_POINTER(tdev->l2opt, NULL);
 	call_rcu(&d->rcu_head, clean_l2_data);
-	if (t->nofail_skb)
-		kfree_skb(t->nofail_skb);
+	kfree_skb(t->nofail_skb);
 	kfree(t);
 }
 

diff --git a/drivers/net/ethernet/chelsio/cxgb3/cxgb3_main.c b/drivers/net/ethernet/chelsio/cxgb3/cxgb3_main.c
index 6505070..0b79309 100644
--- a/drivers/net/ethernet/chelsio/cxgb3/cxgb3_main.c
+++ b/drivers/net/ethernet/chelsio/cxgb3/cxgb3_main.c
@@ -3412,8 +3412,7 @@ static void __devexit remove_one(struct pci_dev *pdev)
 				free_netdev(adapter->port[i]);
 
 		iounmap(adapter->regs);
-		if (adapter->nofail_skb)
-			kfree_skb(adapter->nofail_skb);
+		kfree_skb(adapter->nofail_skb);
 		kfree(adapter);
 		pci_release_regions(pdev);
 		pci_disable_device(pdev);

^ permalink raw reply related

* [PATCH] NFC: remove pointless conditional before kfree_skb()
From: Wei Yongjun @ 2012-08-28 13:02 UTC (permalink / raw)
  To: lauro.venancio, aloisio.almeida, sameo, davem
  Cc: yongjun_wei, linux-wireless, netdev

From: Wei Yongjun <yongjun_wei@trendmicro.com.cn>

Remove pointless conditional before kfree_skb().

Signed-off-by: Wei Yongjun <yongjun_wei@trendmicro.com.cn>
---
 net/nfc/hci/shdlc.c | 3 +--
 net/nfc/hci/core.c  | 3 +--
 2 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/net/nfc/hci/shdlc.c b/net/nfc/hci/shdlc.c
index 6f840c1..52e5cbb 100644
--- a/net/nfc/hci/shdlc.c
+++ b/net/nfc/hci/shdlc.c
@@ -241,8 +241,7 @@ static void nfc_shdlc_rcv_i_frame(struct nfc_shdlc *shdlc,
 	}
 
 exit:
-	if (skb)
-		kfree_skb(skb);
+	kfree_skb(skb);
 }
 
 static void nfc_shdlc_rcv_ack(struct nfc_shdlc *shdlc, int y_nr)

diff --git a/net/nfc/hci/core.c b/net/nfc/hci/core.c
index 1ac7b3f..35413cc 100644
--- a/net/nfc/hci/core.c
+++ b/net/nfc/hci/core.c
@@ -398,8 +398,7 @@ disconnect_all:
 	nfc_hci_disconnect_all_gates(hdev);
 
 exit:
-	if (skb)
-		kfree_skb(skb);
+	kfree_skb(skb);
 
 	return r;
 }

^ permalink raw reply related

* [PATCH] net: qmi_wwan: new device: Foxconn/Novatel E396
From: Aleksander Morgado @ 2012-08-28 12:30 UTC (permalink / raw)
  To: netdev
  Cc: linux-usb, Aleksander Morgado, Dan Williams, Bjørn Mork,
	Ben Chan

Foxconn-branded Novatel E396, Gobi3k modem.

Cc: Dan Williams <dcbw@redhat.com>
Cc: Bjørn Mork <bjorn@mork.no>
Cc: Ben Chan <benchan@google.com>
Signed-off-by: Aleksander Morgado <aleksander@lanedo.com>
---
 drivers/net/usb/qmi_wwan.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/usb/qmi_wwan.c b/drivers/net/usb/qmi_wwan.c
index 328397c..189e52d 100644
--- a/drivers/net/usb/qmi_wwan.c
+++ b/drivers/net/usb/qmi_wwan.c
@@ -441,6 +441,7 @@ static const struct usb_device_id products[] = {
 	{QMI_GOBI_DEVICE(0x1199, 0x9015)},	/* Sierra Wireless Gobi 3000 Modem device */
 	{QMI_GOBI_DEVICE(0x1199, 0x9019)},	/* Sierra Wireless Gobi 3000 Modem device */
 	{QMI_GOBI_DEVICE(0x1199, 0x901b)},	/* Sierra Wireless MC7770 */
+	{QMI_GOBI_DEVICE(0x1410, 0xa021)},	/* Foxconn Gobi 3000 Modem device (Novatel E396) */
 
 	{ }					/* END */
 };
-- 
1.7.11.4

^ permalink raw reply related

* Re: [PATCH v3 01/17] hashtable: introduce a small and naive hashtable
From: Mathieu Desnoyers @ 2012-08-28 11:56 UTC (permalink / raw)
  To: Sasha Levin
  Cc: snitzer-H+wXaHxf7aLQT0dZR+AlfA, neilb-l3A5Bk7waGM,
	fweisbec-Re5JQEeQqe8AvxtiuMwx3w,
	Trond.Myklebust-HgOvQuBEEgTQT0dZR+AlfA,
	bfields-uC3wQj2KruNg9hUCZPvPmw,
	paul.gortmaker-CWA4WttNNZF54TAoqtyWWQ,
	dm-devel-H+wXaHxf7aLQT0dZR+AlfA, agk-H+wXaHxf7aLQT0dZR+AlfA,
	aarcange-H+wXaHxf7aLQT0dZR+AlfA, rds-devel-N0ozoZBvEnrZJqsBc5GL+g,
	eric.dumazet-Re5JQEeQqe8AvxtiuMwx3w,
	venkat.x.venkatsubra-QHcLZuEGTsvQT0dZR+AlfA,
	ccaulfie-H+wXaHxf7aLQT0dZR+AlfA, mingo-X9Un+BFzKDI,
	dev-yBygre7rU0TnMu66kgdUjQ, ericvh-Re5JQEeQqe8AvxtiuMwx3w,
	josh-iaAMLnmF4UmaiuxdJuQwMA, rostedt-nx8X9YLhiw1AfugRpC6u6w,
	lw-BthXqXjhjHXQFUHtdCDX3A, teigland-H+wXaHxf7aLQT0dZR+AlfA,
	axboe-tSWWG44O7X1aa/9Udqfwiw, linux-nfs-u79uwXL29TY76Z2rM5mHXA,
	edumazet-hpIqsD4AKlfQT0dZR+AlfA, linux-mm-Bw31MaZKKs3YtjvyW6yDsg,
	netdev-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, ejt-H+wXaHxf7aLQT0dZR+AlfA,
	ebiederm-aS9lmoZGLiVWk0Htik3J/w, Tejun Heo,
	akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b,
	torvalds-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b,
	davem-fT/PcQaiUtIeIZ0/mPfg9Q
In-Reply-To: <503CAB1E.5010408-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

* Sasha Levin (levinsasha928-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org) wrote:
> On 08/28/2012 12:11 PM, Mathieu Desnoyers wrote:
> > * Sasha Levin (levinsasha928-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org) wrote:
> >> On 08/25/2012 06:24 AM, Mathieu Desnoyers wrote:
> >>> * Tejun Heo (tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org) wrote:
> >>>> Hello,
> >>>>
> >>>> On Sat, Aug 25, 2012 at 12:59:25AM +0200, Sasha Levin wrote:
> >>>>> Thats the thing, the amount of things of things you can do with a given bucket
> >>>>> is very limited. You can't add entries to any point besides the head (without
> >>>>> walking the entire list).
> >>>>
> >>>> Kinda my point.  We already have all the hlist*() interface to deal
> >>>> with such cases.  Having something which is evidently the trivial
> >>>> hlist hashtable and advertises as such in the interface can be
> >>>> helpful.  I think we need that more than we need anything fancy.
> >>>>
> >>>> Heh, this is a debate about which one is less insignificant.  I can
> >>>> see your point.  I'd really like to hear what others think on this.
> >>>>
> >>>> Guys, do we want something which is evidently trivial hlist hashtable
> >>>> which can use hlist_*() API directly or do we want something better
> >>>> encapsulated?
> >>>
> >>> My 2 cents, FWIW: I think this specific effort should target a trivially
> >>> understandable API and implementation, for use-cases where one would be
> >>> tempted to reimplement his own trivial hash table anyway. So here
> >>> exposing hlist internals, with which kernel developers are already
> >>> familiar, seems like a good approach in my opinion, because hiding stuff
> >>> behind new abstraction might make the target users go away.
> >>>
> >>> Then, as we see the need, we can eventually merge a more elaborate hash
> >>> table with poneys and whatnot, but I would expect that the trivial hash
> >>> table implementation would still be useful. There are of course very
> >>> compelling reasons to use a more featureful hash table: automatic
> >>> resize, RT-aware updates, scalable updates, etc... but I see a purpose
> >>> for a trivial implementation. Its primary strong points being:
> >>>
> >>> - it's trivially understandable, so anyone how want to be really sure
> >>>   they won't end up debugging the hash table instead of their
> >>>   work-in-progress code can have a full understanding of it,
> >>> - it has few dependencies, which makes it easier to understand and
> >>>   easier to use in some contexts (e.g. early boot).
> >>>
> >>> So I'm in favor of not overdoing the abstraction for this trivial hash
> >>> table, and honestly I would rather prefer that this trivial hash table
> >>> stays trivial. A more elaborate hash table should probably come as a
> >>> separate API.
> >>>
> >>> Thanks,
> >>>
> >>> Mathieu
> >>>
> >>
> >> Alright, let's keep it simple then.
> >>
> >> I do want to keep the hash_for_each[rcu,safe] family though.
> > 
> > Just a thought: if the API offered by the simple hash table focus on
> > providing a mechanism to find the hash bucket to which belongs the hash
> > chain containing the key looked up, and then expects the user to use the
> > hlist API to iterate on the chain (with or without the hlist _rcu
> > variant), then it might seem consistent that a helper providing
> > iteration over the entire table would actually just provide iteration on
> > all buckets, and let the user call the hlist for each iterator for each
> > node within the bucket, e.g.:
> > 
> > struct hlist_head *head;
> > struct hlist_node *pos;
> > 
> > hash_for_each_bucket(ht, head) {
> >         hlist_for_each(pos, head) {
> >                 ...
> >         }
> > }
> > 
> > That way you only have to provide one single macro
> > (hash_for_each_bucket), and rely on the already existing:
> > 
> > - hlist_for_each_entry
> > - hlist_for_each_safe
> > - hlist_for_each_entry_rcu
> > - hlist_for_each_safe_rcu
> >   .....
> > 
> > and various flavors that can appear in the future without duplicating
> > this API. So you won't even have to create _rcu, _safe, nor _safe_rcu
> > versions of the hash_for_each_bucket macro.
> > 
> > Thoughts ?
> 
> In my opinion, the downside here is that it'll require 2 function calls and 2
> levels of nesting for a simple hash iteration.

Those are macros, not functions. No function call is required. But I see
your point about nesting.

> 
> hash_for_each_bucket() will always be followed by an iteration of that
> bucket, so splitting a hash_for_each() which does both into 2
> different functions which will almost always must be called in that
> given order sounds unintuitive to me.
> 
> It's also just 3 different possible iterators:
> 
>  - hlist_for_each_entry
>  - hlist_for_each_entry_safe
>  - hlist_for_each_entry_rcu
> 
> So I think that it's a good price to pay - 2 extra macro definitions
> in the header to save a macro call + nesting level in each place that
> uses a hashtable.

I must admin I don't care that much one way or another.

Thanks,

Mathieu

> 
> 
> Thanks,
> Sasha
> 
> > Thanks,
> > 
> > Mathieu
> > 
> 

-- 
Mathieu Desnoyers
Operating System Efficiency R&D Consultant
EfficiOS Inc.
http://www.efficios.com

^ permalink raw reply

* Re: [PATCH v3 01/17] hashtable: introduce a small and naive hashtable
From: Sasha Levin @ 2012-08-28 11:27 UTC (permalink / raw)
  To: Mathieu Desnoyers
  Cc: Tejun Heo, torvalds-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b,
	akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-mm-Bw31MaZKKs3YtjvyW6yDsg,
	paul.gortmaker-CWA4WttNNZF54TAoqtyWWQ,
	davem-fT/PcQaiUtIeIZ0/mPfg9Q, rostedt-nx8X9YLhiw1AfugRpC6u6w,
	mingo-X9Un+BFzKDI, ebiederm-aS9lmoZGLiVWk0Htik3J/w,
	aarcange-H+wXaHxf7aLQT0dZR+AlfA, ericvh-Re5JQEeQqe8AvxtiuMwx3w,
	netdev-u79uwXL29TY76Z2rM5mHXA, josh-iaAMLnmF4UmaiuxdJuQwMA,
	eric.dumazet-Re5JQEeQqe8AvxtiuMwx3w, axboe-tSWWG44O7X1aa/9Udqfwiw,
	agk-H+wXaHxf7aLQT0dZR+AlfA, dm-devel-H+wXaHxf7aLQT0dZR+AlfA,
	neilb-l3A5Bk7waGM, ccaulfie-H+wXaHxf7aLQT0dZR+AlfA,
	teigland-H+wXaHxf7aLQT0dZR+AlfA,
	Trond.Myklebust-HgOvQuBEEgTQT0dZR+AlfA,
	bfields-uC3wQj2KruNg9hUCZPvPmw, fweisbec-Re5JQEeQqe8AvxtiuMwx3w,
	jesse-l0M0P4e3n4LQT0dZR+AlfA,
	venkat.x.venkatsubra-QHcLZuEGTsvQT0dZR+AlfA,
	ejt-H+wXaHxf7aLQT0dZR+AlfA, snitzer-H+wXaHxf7aLQT0dZR+AlfA,
	edumazet-hpIqsD4AKlfQT0dZR+AlfA, linux-nfs-u79uwXL29TY76Z2rM5mHXA,
	dev-yBygre7rU0TnMu66kgdUjQ, rds-devel-N0ozoZBvEnrZJqsBc5GL+g,
	lw-BthXqXjhjHXQFUHtdCDX3A
In-Reply-To: <20120828101148.GA21683@Krystal>

On 08/28/2012 12:11 PM, Mathieu Desnoyers wrote:
> * Sasha Levin (levinsasha928-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org) wrote:
>> On 08/25/2012 06:24 AM, Mathieu Desnoyers wrote:
>>> * Tejun Heo (tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org) wrote:
>>>> Hello,
>>>>
>>>> On Sat, Aug 25, 2012 at 12:59:25AM +0200, Sasha Levin wrote:
>>>>> Thats the thing, the amount of things of things you can do with a given bucket
>>>>> is very limited. You can't add entries to any point besides the head (without
>>>>> walking the entire list).
>>>>
>>>> Kinda my point.  We already have all the hlist*() interface to deal
>>>> with such cases.  Having something which is evidently the trivial
>>>> hlist hashtable and advertises as such in the interface can be
>>>> helpful.  I think we need that more than we need anything fancy.
>>>>
>>>> Heh, this is a debate about which one is less insignificant.  I can
>>>> see your point.  I'd really like to hear what others think on this.
>>>>
>>>> Guys, do we want something which is evidently trivial hlist hashtable
>>>> which can use hlist_*() API directly or do we want something better
>>>> encapsulated?
>>>
>>> My 2 cents, FWIW: I think this specific effort should target a trivially
>>> understandable API and implementation, for use-cases where one would be
>>> tempted to reimplement his own trivial hash table anyway. So here
>>> exposing hlist internals, with which kernel developers are already
>>> familiar, seems like a good approach in my opinion, because hiding stuff
>>> behind new abstraction might make the target users go away.
>>>
>>> Then, as we see the need, we can eventually merge a more elaborate hash
>>> table with poneys and whatnot, but I would expect that the trivial hash
>>> table implementation would still be useful. There are of course very
>>> compelling reasons to use a more featureful hash table: automatic
>>> resize, RT-aware updates, scalable updates, etc... but I see a purpose
>>> for a trivial implementation. Its primary strong points being:
>>>
>>> - it's trivially understandable, so anyone how want to be really sure
>>>   they won't end up debugging the hash table instead of their
>>>   work-in-progress code can have a full understanding of it,
>>> - it has few dependencies, which makes it easier to understand and
>>>   easier to use in some contexts (e.g. early boot).
>>>
>>> So I'm in favor of not overdoing the abstraction for this trivial hash
>>> table, and honestly I would rather prefer that this trivial hash table
>>> stays trivial. A more elaborate hash table should probably come as a
>>> separate API.
>>>
>>> Thanks,
>>>
>>> Mathieu
>>>
>>
>> Alright, let's keep it simple then.
>>
>> I do want to keep the hash_for_each[rcu,safe] family though.
> 
> Just a thought: if the API offered by the simple hash table focus on
> providing a mechanism to find the hash bucket to which belongs the hash
> chain containing the key looked up, and then expects the user to use the
> hlist API to iterate on the chain (with or without the hlist _rcu
> variant), then it might seem consistent that a helper providing
> iteration over the entire table would actually just provide iteration on
> all buckets, and let the user call the hlist for each iterator for each
> node within the bucket, e.g.:
> 
> struct hlist_head *head;
> struct hlist_node *pos;
> 
> hash_for_each_bucket(ht, head) {
>         hlist_for_each(pos, head) {
>                 ...
>         }
> }
> 
> That way you only have to provide one single macro
> (hash_for_each_bucket), and rely on the already existing:
> 
> - hlist_for_each_entry
> - hlist_for_each_safe
> - hlist_for_each_entry_rcu
> - hlist_for_each_safe_rcu
>   .....
> 
> and various flavors that can appear in the future without duplicating
> this API. So you won't even have to create _rcu, _safe, nor _safe_rcu
> versions of the hash_for_each_bucket macro.
> 
> Thoughts ?

In my opinion, the downside here is that it'll require 2 function calls and 2
levels of nesting for a simple hash iteration.

hash_for_each_bucket() will always be followed by an iteration of that bucket,
so splitting a hash_for_each() which does both into 2 different functions which
will almost always must be called in that given order sounds unintuitive to me.

It's also just 3 different possible iterators:

 - hlist_for_each_entry
 - hlist_for_each_entry_safe
 - hlist_for_each_entry_rcu

So I think that it's a good price to pay - 2 extra macro definitions in the
header to save a macro call + nesting level in each place that uses a hashtable.


Thanks,
Sasha

> Thanks,
> 
> Mathieu
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: CBQ(but probably u32 filter bug), kernel "freeze", at least from 3.2.0
From: Eric Dumazet @ 2012-08-28 10:59 UTC (permalink / raw)
  To: Denys Fedoryshchenko; +Cc: netdev
In-Reply-To: <a680a8297b2f099eb713b266792b9b82@visp.net.lb>

On Tue, 2012-08-28 at 07:50 +0300, Denys Fedoryshchenko wrote:
> Hi
> 
> Got information from friend, confirmed that it crashed at least two my 
> boxes also :)
> 3.0.5-rc1 is working fine, 3.4.1 , 3.2.0 from ubuntu  - crashing
> No watchdog fired, and didn't got yet significant debugging 
> information.
> 
> Very easy to reproduce:
> 1)run the script
> 2)ping 192.168.3.234
> 
> script:
> DEV_OUT=eth0
> ICMP="match ip protocol 1 0xff"
> U32="protocol ip u32"
> DST="match ip dst"
> tc qdisc add dev $DEV_OUT root handle 1: cbq avpkt 1000 bandwidth 
> 100mbit
> tc class add dev $DEV_OUT parent 1: classid 1:1 cbq rate 512kbit allot 
> 1500 prio 5 bounded isolated
> tc filter add dev $DEV_OUT parent 1:              prio 3 $U32 $ICMP 
> $DST 192.168.3.234 flowid 1:
> tc qdisc add dev $DEV_OUT parent 1:1 sfq perturb 10

Not sure what your friend expected from this buggy configuration.

It probably never worked at all.

CBQ needs at least one child class and one leaf class.

This scripts creates a loop inside CBQ, so cpu is probably looping in
cbq_enqueue() (or more exactly cbq_classify()), as instructed by the
sysadmin ;)

u32 (or sfq) seems ok.

Could you try the following patch ?

Thanks !

diff --git a/net/sched/sch_cbq.c b/net/sched/sch_cbq.c
index 6aabd77..564b9fc 100644
--- a/net/sched/sch_cbq.c
+++ b/net/sched/sch_cbq.c
@@ -250,10 +250,11 @@ cbq_classify(struct sk_buff *skb, struct Qdisc *sch, int *qerr)
 			else if ((cl = defmap[res.classid & TC_PRIO_MAX]) == NULL)
 				cl = defmap[TC_PRIO_BESTEFFORT];
 
-			if (cl == NULL || cl->level >= head->level)
+			if (cl == NULL)
 				goto fallback;
 		}
-
+		if (cl->level >= head->level)
+			goto fallback;
 #ifdef CONFIG_NET_CLS_ACT
 		switch (result) {
 		case TC_ACT_QUEUED:

^ permalink raw reply related

* Re: [PATCH v3 01/17] hashtable: introduce a small and naive hashtable
From: Mathieu Desnoyers @ 2012-08-28 10:11 UTC (permalink / raw)
  To: Sasha Levin
  Cc: Tejun Heo, torvalds, akpm, linux-kernel, linux-mm, paul.gortmaker,
	davem, rostedt, mingo, ebiederm, aarcange, ericvh, netdev, josh,
	eric.dumazet, axboe, agk, dm-devel, neilb, ccaulfie, teigland,
	Trond.Myklebust, bfields, fweisbec, jesse, venkat.x.venkatsubra,
	ejt, snitzer, edumazet, linux-nfs, dev, rds-devel, lw
In-Reply-To: <503C95E4.3010000@gmail.com>

* Sasha Levin (levinsasha928@gmail.com) wrote:
> On 08/25/2012 06:24 AM, Mathieu Desnoyers wrote:
> > * Tejun Heo (tj@kernel.org) wrote:
> >> Hello,
> >>
> >> On Sat, Aug 25, 2012 at 12:59:25AM +0200, Sasha Levin wrote:
> >>> Thats the thing, the amount of things of things you can do with a given bucket
> >>> is very limited. You can't add entries to any point besides the head (without
> >>> walking the entire list).
> >>
> >> Kinda my point.  We already have all the hlist*() interface to deal
> >> with such cases.  Having something which is evidently the trivial
> >> hlist hashtable and advertises as such in the interface can be
> >> helpful.  I think we need that more than we need anything fancy.
> >>
> >> Heh, this is a debate about which one is less insignificant.  I can
> >> see your point.  I'd really like to hear what others think on this.
> >>
> >> Guys, do we want something which is evidently trivial hlist hashtable
> >> which can use hlist_*() API directly or do we want something better
> >> encapsulated?
> > 
> > My 2 cents, FWIW: I think this specific effort should target a trivially
> > understandable API and implementation, for use-cases where one would be
> > tempted to reimplement his own trivial hash table anyway. So here
> > exposing hlist internals, with which kernel developers are already
> > familiar, seems like a good approach in my opinion, because hiding stuff
> > behind new abstraction might make the target users go away.
> > 
> > Then, as we see the need, we can eventually merge a more elaborate hash
> > table with poneys and whatnot, but I would expect that the trivial hash
> > table implementation would still be useful. There are of course very
> > compelling reasons to use a more featureful hash table: automatic
> > resize, RT-aware updates, scalable updates, etc... but I see a purpose
> > for a trivial implementation. Its primary strong points being:
> > 
> > - it's trivially understandable, so anyone how want to be really sure
> >   they won't end up debugging the hash table instead of their
> >   work-in-progress code can have a full understanding of it,
> > - it has few dependencies, which makes it easier to understand and
> >   easier to use in some contexts (e.g. early boot).
> > 
> > So I'm in favor of not overdoing the abstraction for this trivial hash
> > table, and honestly I would rather prefer that this trivial hash table
> > stays trivial. A more elaborate hash table should probably come as a
> > separate API.
> > 
> > Thanks,
> > 
> > Mathieu
> > 
> 
> Alright, let's keep it simple then.
> 
> I do want to keep the hash_for_each[rcu,safe] family though.

Just a thought: if the API offered by the simple hash table focus on
providing a mechanism to find the hash bucket to which belongs the hash
chain containing the key looked up, and then expects the user to use the
hlist API to iterate on the chain (with or without the hlist _rcu
variant), then it might seem consistent that a helper providing
iteration over the entire table would actually just provide iteration on
all buckets, and let the user call the hlist for each iterator for each
node within the bucket, e.g.:

struct hlist_head *head;
struct hlist_node *pos;

hash_for_each_bucket(ht, head) {
        hlist_for_each(pos, head) {
                ...
        }
}

That way you only have to provide one single macro
(hash_for_each_bucket), and rely on the already existing:

- hlist_for_each_entry
- hlist_for_each_safe
- hlist_for_each_entry_rcu
- hlist_for_each_safe_rcu
  .....

and various flavors that can appear in the future without duplicating
this API. So you won't even have to create _rcu, _safe, nor _safe_rcu
versions of the hash_for_each_bucket macro.

Thoughts ?

Thanks,

Mathieu

-- 
Mathieu Desnoyers
Operating System Efficiency R&D Consultant
EfficiOS Inc.
http://www.efficios.com

^ permalink raw reply

* Re: [PATCH v3 01/17] hashtable: introduce a small and naive hashtable
From: Sasha Levin @ 2012-08-28  9:56 UTC (permalink / raw)
  To: Mathieu Desnoyers
  Cc: Tejun Heo, torvalds-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b,
	akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-mm-Bw31MaZKKs3YtjvyW6yDsg,
	paul.gortmaker-CWA4WttNNZF54TAoqtyWWQ,
	davem-fT/PcQaiUtIeIZ0/mPfg9Q, rostedt-nx8X9YLhiw1AfugRpC6u6w,
	mingo-X9Un+BFzKDI, ebiederm-aS9lmoZGLiVWk0Htik3J/w,
	aarcange-H+wXaHxf7aLQT0dZR+AlfA, ericvh-Re5JQEeQqe8AvxtiuMwx3w,
	netdev-u79uwXL29TY76Z2rM5mHXA, josh-iaAMLnmF4UmaiuxdJuQwMA,
	eric.dumazet-Re5JQEeQqe8AvxtiuMwx3w, axboe-tSWWG44O7X1aa/9Udqfwiw,
	agk-H+wXaHxf7aLQT0dZR+AlfA, dm-devel-H+wXaHxf7aLQT0dZR+AlfA,
	neilb-l3A5Bk7waGM, ccaulfie-H+wXaHxf7aLQT0dZR+AlfA,
	teigland-H+wXaHxf7aLQT0dZR+AlfA,
	Trond.Myklebust-HgOvQuBEEgTQT0dZR+AlfA,
	bfields-uC3wQj2KruNg9hUCZPvPmw, fweisbec-Re5JQEeQqe8AvxtiuMwx3w,
	jesse-l0M0P4e3n4LQT0dZR+AlfA,
	venkat.x.venkatsubra-QHcLZuEGTsvQT0dZR+AlfA,
	ejt-H+wXaHxf7aLQT0dZR+AlfA, snitzer-H+wXaHxf7aLQT0dZR+AlfA,
	edumazet-hpIqsD4AKlfQT0dZR+AlfA, linux-nfs-u79uwXL29TY76Z2rM5mHXA,
	dev-yBygre7rU0TnMu66kgdUjQ, rds-devel-N0ozoZBvEnrZJqsBc5GL+g,
	lw-BthXqXjhjHXQFUHtdCDX3A
In-Reply-To: <20120825042419.GA27240@Krystal>

On 08/25/2012 06:24 AM, Mathieu Desnoyers wrote:
> * Tejun Heo (tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org) wrote:
>> Hello,
>>
>> On Sat, Aug 25, 2012 at 12:59:25AM +0200, Sasha Levin wrote:
>>> Thats the thing, the amount of things of things you can do with a given bucket
>>> is very limited. You can't add entries to any point besides the head (without
>>> walking the entire list).
>>
>> Kinda my point.  We already have all the hlist*() interface to deal
>> with such cases.  Having something which is evidently the trivial
>> hlist hashtable and advertises as such in the interface can be
>> helpful.  I think we need that more than we need anything fancy.
>>
>> Heh, this is a debate about which one is less insignificant.  I can
>> see your point.  I'd really like to hear what others think on this.
>>
>> Guys, do we want something which is evidently trivial hlist hashtable
>> which can use hlist_*() API directly or do we want something better
>> encapsulated?
> 
> My 2 cents, FWIW: I think this specific effort should target a trivially
> understandable API and implementation, for use-cases where one would be
> tempted to reimplement his own trivial hash table anyway. So here
> exposing hlist internals, with which kernel developers are already
> familiar, seems like a good approach in my opinion, because hiding stuff
> behind new abstraction might make the target users go away.
> 
> Then, as we see the need, we can eventually merge a more elaborate hash
> table with poneys and whatnot, but I would expect that the trivial hash
> table implementation would still be useful. There are of course very
> compelling reasons to use a more featureful hash table: automatic
> resize, RT-aware updates, scalable updates, etc... but I see a purpose
> for a trivial implementation. Its primary strong points being:
> 
> - it's trivially understandable, so anyone how want to be really sure
>   they won't end up debugging the hash table instead of their
>   work-in-progress code can have a full understanding of it,
> - it has few dependencies, which makes it easier to understand and
>   easier to use in some contexts (e.g. early boot).
> 
> So I'm in favor of not overdoing the abstraction for this trivial hash
> table, and honestly I would rather prefer that this trivial hash table
> stays trivial. A more elaborate hash table should probably come as a
> separate API.
> 
> Thanks,
> 
> Mathieu
> 

Alright, let's keep it simple then.

I do want to keep the hash_for_each[rcu,safe] family though.

--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH 2/2] ipvs: Extend MTU check to account for IPv6 NAT defrag changes
From: Julian Anastasov @ 2012-08-28  9:47 UTC (permalink / raw)
  To: Jesper Dangaard Brouer
  Cc: netdev, Patrick McHardy, lvs-devel, Simon Horman,
	Pablo Neira Ayuso, Hans Schillstrom, Wensong Zhang,
	netfilter-devel
In-Reply-To: <1346144607.3069.415.camel@localhost>


	Hello,

On Tue, 28 Aug 2012, Jesper Dangaard Brouer wrote:

> On Mon, 2012-08-27 at 18:20 +0300, Julian Anastasov wrote:
> > 
> > > @@ -956,8 +963,11 @@ ip_vs_tunnel_xmit_v6(struct sk_buff *skb, struct ip_vs_conn *cp,
> > >  		skb_dst(skb)->ops->update_pmtu(skb_dst(skb), NULL, skb, mtu);
> > >  
> > >  	/* MTU checking: Special for tunnel mode */
> > > -	if (mtu < ntohs(old_iph->payload_len) + sizeof(struct ipv6hdr) &&
> 
> I guess:
>  ntohs(old_iph->payload_len) + sizeof(struct ipv6hdr)
> Is the same as:
>  skb->len

	I think so. You can think of this in different way:
all transmitters are called from same place, there is no
difference in the packets we see. When we can use
__mtu_check_toobig_v6 for other methods relying on skb->len
being correct, we can do the same for tunnels, only that
tunnels have lower MTU, that is the only difference.

> > > -	    !skb_is_gso(skb)) {
> > > +	if ((!IP6CB(skb)->frag_max_size &&
> > > +	     (mtu < ntohs(old_iph->payload_len) + sizeof(struct ipv6hdr) &&
> > > +	      !skb_is_gso(skb)))
> > > +	    || IP6CB(skb)->frag_max_size + sizeof(struct ipv6hdr) > mtu) {
> 
> > 
> > 	mtu is already reduced with the new outer header size,
> > may be we can just call __mtu_check_toobig_v6 with mtu?
> 
> To Julian, is the extra sizeof(struct ipv6hdr) addition to
> frag_max_size, wrong? (as the mtu is already reduced)

	Yes, sizeof(struct ipv6hdr) is needed only together
with payload_len because payload_len does not include the
first header.

> If above statements hold, I think we can simply use
> __mtu_check_toobig_v6() also for the tunnel case :-)

	Yep

> --Jesper

Regards

--
Julian Anastasov <ja@ssi.bg>

^ permalink raw reply

* Re: [PATCH] userns: Add basic quota support v2
From: Boaz Harrosh @ 2012-08-28  9:44 UTC (permalink / raw)
  To: Jan Kara
  Cc: Eric W. Biederman, linux-kernel, netdev, linux-fsdevel,
	Serge E. Hallyn, David Miller, Steven Whitehouse, Mark Fasheh,
	Joel Becker, Ben Myers, Alex Elder, Dmitry Monakhov, Abhijith Das
In-Reply-To: <20120828090544.GC5146@quack.suse.cz>

On 08/28/2012 12:05 PM, Jan Kara wrote:

>> +static inline u32 from_qown(struct user_namespace *user_ns, struct qown qown)
>> +{
>> +	switch (qown.type) {
>> +	case USRQUOTA:
>> +		return from_kuid(user_ns, qown.uid);
>> +	case GRPQUOTA:
>> +		return from_kgid(user_ns, qown.gid);
>> +	case XQM_PRJQUOTA:
>> +		return (user_ns == &init_user_ns) ? qown.prj : -1;
>> +	default:
>> +		BUG();
>> +	}
>> +}
>   I would like a bit more if the function somehow expressed in its name
> that it returns id. id_from_qown() might be a bit too long given how often
> it is used. qown2id() would be OK but it would be inconsistent with how
> names of other functions you've added are formed. So I'm somewhat
> undecided...
> 


qown_id()

> 								Honza

^ permalink raw reply

* [PATCH 1/1] net: fix documentation of skb_needs_linearize().
From: Rami Rosen @ 2012-08-28  9:39 UTC (permalink / raw)
  To: davem; +Cc: netdev, Rami Rosen

skb_needs_linearize() does not check highmem DMA as it does not call
illegal_highdma() anymore, so there is no need to mention highmem DMA here.

(Indeed, ~NETIF_F_SG flag, which is checked in skb_needs_linearize(), can
be set when illegal_highdma() returns true, and we are assured that
illegal_highdma() is invoked prior to skb_needs_linearize() as
skb_needs_linearize() is a static method called only once.
But ~NETIF_F_SG can be set not only there in this same invocation path.
It can also be set when can_checksum_protocol() returns false).

see commit 02932ce9e2c136e6fab2571c8e0dd69ae8ec9853,
Convert skb_need_linearize() to use precomputed features.
Signed-off-by: Rami Rosen <rosenr@marvell.com>
---
 net/core/dev.c |    4 +---
 1 files changed, 1 insertions(+), 3 deletions(-)

diff --git a/net/core/dev.c b/net/core/dev.c
index 3401e2d..a4da9d8 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -2184,9 +2184,7 @@ EXPORT_SYMBOL(netif_skb_features);
 /*
  * Returns true if either:
  *	1. skb has frag_list and the device doesn't support FRAGLIST, or
- *	2. skb is fragmented and the device does not support SG, or if
- *	   at least one of fragments is in highmem and device does not
- *	   support DMA from it.
+ *	2. skb is fragmented and the device does not support SG.
  */
 static inline int skb_needs_linearize(struct sk_buff *skb,
 				      int features)
-- 
1.7.7.6

^ permalink raw reply related

* 254....654...651--2..
From: wxo @ 2012-08-16  9:34 UTC (permalink / raw)
  To: service

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="gb2312", Size: 247 bytes --]



ÄúºÃ£¡

       ÎÒ˾¿É´ú¿ªÈ«¹ú¸÷µØÆÕͨ(¹ú-µØË°)(·¢*Ʊ)£¬

µãÊýÓŻݣ¬³Ðŵ¾ø¶Ô(Õæ--Ʊ)£¬¿ÉÑéÖ¤ºóÔÙ¸¶¿î¡£

£¨ÈôÓдòÈÅ£¬ÇëÄúÁ½⣩¡£Ð»Ð»!

        
          “ϵÈË: ÁÖС½ã

           ÊÖ ™C: 13530058313

             Q Q: 510951266 

^ permalink raw reply

* Re: [PATCH] userns: Add basic quota support v2
From: Jan Kara @ 2012-08-28  9:05 UTC (permalink / raw)
  To: Eric W. Biederman
  Cc: Jan Kara, linux-kernel, netdev, linux-fsdevel, Serge E. Hallyn,
	David Miller, Steven Whitehouse, Mark Fasheh, Joel Becker,
	Ben Myers, Alex Elder, Dmitry Monakhov, Abhijith Das
In-Reply-To: <87wr0j7u3j.fsf_-_@xmission.com>

On Mon 27-08-12 17:12:16, Eric W. Biederman wrote: 
> Add the data type struct qown which holds the owning identifier of a
> quota.  struct qown is a replacement for the implicit union of uid,
> gid and project stored in an unsigned int and the quota type field
> that is was used in the quota data structures.  Making the data type
> explicit allows the kuid_t and kgid_t type safety to propogate more
> thoroughly through the code, revealing more places where uid/gid
> conversions need be made.
> 
> Allong with the data type struct qown comes the helper functions
  ^^^^ Along

> qown_eq, qown_lt, from_qown, from_qown_munged, qown_valid, make_qown,
> make_qown_invalid, make_qown_uid, make_qown_gid.
> 
> Replace struct dquot dq_id and dq_type with dq_own a struct qown.
> 
> Update the signature of dqget, quota_send_warning, dquot_get_dqblk,
> and dquot_set_dqblk to use struct qown.
> 
> Make minimal changes to ext3, ext4, gfs2, ocfs2, and xfs to deal with
> the change in quota structures and signatures.  The ocfs2 changes are
> larger than most because of the extensive tracing throughout the ocfs2
> quota code that prints out dq_id.
> 
> v2:
>  - Renamed qown_t struct qown
>  - Added the quota type to struct qown.
>  - Removed enum quota_type (In this patch it was just noise)
>  - Added qown_lt, make_qown_invalid, make_qown_uid, make_qown_gid
>  - Taught qown to handle xfs project ids (but only in init_user_ns). 
>    Q_XGETQUOTA calls .get_quotblk with project ids.
  Just a couple one minor comments below...

> @@ -130,13 +130,17 @@ static void copy_to_if_dqblk(struct if_dqblk *dst, struct fs_disk_quota *src)
>  static int quota_getquota(struct super_block *sb, int type, qid_t id,
>  			  void __user *addr)
>  {
> +	struct qown qown;
>  	struct fs_disk_quota fdq;
>  	struct if_dqblk idq;
>  	int ret;
>  
>  	if (!sb->s_qcop->get_dqblk)
>  		return -ENOSYS;
> -	ret = sb->s_qcop->get_dqblk(sb, type, id, &fdq);
> +	qown = make_qown(current_user_ns(), type, id);
> +	if (qown_valid(qown))
            ^ missing '!'

> +		return -EINVAL;
> +	ret = sb->s_qcop->get_dqblk(sb, qown, &fdq);
>  	if (ret)
>  		return ret;
>  	copy_to_if_dqblk(&idq, &fdq);
...
> +static inline u32 from_qown(struct user_namespace *user_ns, struct qown qown)
> +{
> +	switch (qown.type) {
> +	case USRQUOTA:
> +		return from_kuid(user_ns, qown.uid);
> +	case GRPQUOTA:
> +		return from_kgid(user_ns, qown.gid);
> +	case XQM_PRJQUOTA:
> +		return (user_ns == &init_user_ns) ? qown.prj : -1;
> +	default:
> +		BUG();
> +	}
> +}
  I would like a bit more if the function somehow expressed in its name
that it returns id. id_from_qown() might be a bit too long given how often
it is used. qown2id() would be OK but it would be inconsistent with how
names of other functions you've added are formed. So I'm somewhat
undecided...

								Honza
-- 
Jan Kara <jack@suse.cz>
SUSE Labs, CR

^ permalink raw reply

* Re: [PATCH 2/2] ipvs: Extend MTU check to account for IPv6 NAT defrag changes
From: Jesper Dangaard Brouer @ 2012-08-28  9:03 UTC (permalink / raw)
  To: Julian Anastasov
  Cc: netdev, Patrick McHardy, lvs-devel, Simon Horman,
	Pablo Neira Ayuso, Hans Schillstrom, Wensong Zhang,
	netfilter-devel
In-Reply-To: <alpine.LFD.2.00.1208271810070.24809@ja.ssi.bg>

On Mon, 2012-08-27 at 18:20 +0300, Julian Anastasov wrote:
> 	Hello,
> 
> On Mon, 27 Aug 2012, Jesper Dangaard Brouer wrote:
> 
> > This patch is necessary, to make IPVS work, after Patrick McHardys
> > IPv6 NAT defragmentation changes.
> > 
> > Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com>
> > ---
> > 
> > I would appriciate, if someone (e.g. Julian) double check the tunnel mode code.
> 
> 	Sure
> 
> > --Jesper
> > 
> >  net/netfilter/ipvs/ip_vs_xmit.c |   16 +++++++++++++---
> >  1 files changed, 13 insertions(+), 3 deletions(-)
> > 
> > @@ -956,8 +963,11 @@ ip_vs_tunnel_xmit_v6(struct sk_buff *skb, struct ip_vs_conn *cp,
> >  		skb_dst(skb)->ops->update_pmtu(skb_dst(skb), NULL, skb, mtu);
> >  
> >  	/* MTU checking: Special for tunnel mode */
> > -	if (mtu < ntohs(old_iph->payload_len) + sizeof(struct ipv6hdr) &&

I guess:
 ntohs(old_iph->payload_len) + sizeof(struct ipv6hdr)
Is the same as:
 skb->len

> > -	    !skb_is_gso(skb)) {
> > +	if ((!IP6CB(skb)->frag_max_size &&
> > +	     (mtu < ntohs(old_iph->payload_len) + sizeof(struct ipv6hdr) &&
> > +	      !skb_is_gso(skb)))
> > +	    || IP6CB(skb)->frag_max_size + sizeof(struct ipv6hdr) > mtu) {

> 
> 	mtu is already reduced with the new outer header size,
> may be we can just call __mtu_check_toobig_v6 with mtu?

To Julian, is the extra sizeof(struct ipv6hdr) addition to
frag_max_size, wrong? (as the mtu is already reduced)

If above statements hold, I think we can simply use
__mtu_check_toobig_v6() also for the tunnel case :-)

--Jesper


^ permalink raw reply

* [PATCH net-next] net: ipv4: optimize tkey_mismatch
From: igorm @ 2012-08-28  8:52 UTC (permalink / raw)
  To: netdev; +Cc: davem, Igor Maravic
In-Reply-To: <1346143938-17167-1-git-send-email-igorm@etf.rs>

From: Igor Maravic <igorm@etf.rs>

Optimize tkey_mismatch function by using __fls function.

Signed-off-by: Igor Maravic <igorm@etf.rs>
---
 net/ipv4/fib_trie.c |   10 ++--------
 1 file changed, 2 insertions(+), 8 deletions(-)

diff --git a/net/ipv4/fib_trie.c b/net/ipv4/fib_trie.c
index ec45621..b4b126c 100644
--- a/net/ipv4/fib_trie.c
+++ b/net/ipv4/fib_trie.c
@@ -265,14 +265,8 @@ static inline int tkey_sub_equals(t_key a, int offset, int bits, t_key b)
 
 static inline int tkey_mismatch(t_key a, int offset, t_key b)
 {
-	t_key diff = a ^ b;
-	int i = offset;
-
-	if (!diff)
-		return 0;
-	while ((diff << i) >> (KEYLENGTH-1) == 0)
-		i++;
-	return i;
+	t_key diff = (a ^ b) & (~((t_key)0) << offset >> offset);
+	return diff ? (KEYLENGTH - __fls(diff) - 1) : 0;
 }
 
 /*
-- 
1.7.9.5

^ permalink raw reply related

* [PATCH net-next] net: ipv4: Optimize check_leaf function
From: igorm @ 2012-08-28  8:52 UTC (permalink / raw)
  To: netdev; +Cc: davem, Igor Maravic

From: Igor Maravic <igorm@etf.rs>

If the found tos is smaller then our tos,
don't pass through all the entries in fib_alias
list to find the match.
Directly jump to the last entry in the list, and check if the
tos value is 0.

Signed-off-by: Igor Maravic <igorm@etf.rs>
---
 net/ipv4/fib_trie.c |   10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/net/ipv4/fib_trie.c b/net/ipv4/fib_trie.c
index 3c820da..ec45621 100644
--- a/net/ipv4/fib_trie.c
+++ b/net/ipv4/fib_trie.c
@@ -1366,8 +1366,14 @@ static int check_leaf(struct fib_table *tb, struct trie *t, struct leaf *l,
 			struct fib_info *fi = fa->fa_info;
 			int nhsel, err;
 
-			if (fa->fa_tos && fa->fa_tos != flp->flowi4_tos)
-				continue;
+			if (fa->fa_tos && fa->fa_tos != flp->flowi4_tos) {
+				if (fa->fa_tos < flp->flowi4_tos) {
+					fa = list_entry_rcu(li->falh.prev, struct fib_alias, fa_list);
+					if (fa->fa_tos)
+						break;
+				} else
+					continue;
+			}
 			if (fi->fib_dead)
 				continue;
 			if (fa->fa_info->fib_scope < flp->flowi4_scope)
-- 
1.7.9.5

^ permalink raw reply related

* Re: [PATCH 1/1] tcp: Wrong timeout for SYN segments
From: Carsten Wolff @ 2012-08-28  8:44 UTC (permalink / raw)
  To: Alexander Bergmann
  Cc: David Miller, eric.dumazet, hkjerry.chu, netdev, linux-kernel
In-Reply-To: <20120825084819.GD430@linlab.net>

Hi,

On Saturday 25 August 2012, Alexander Bergmann wrote:
> This patch increases the value of TCP_SYN_RETRIES to the value of 6,
> providing a retransmission window of 63secs.
> 
> The comments for SYN and SYNACK retries have also been updated to
> describe the current settings.

wouldn't it be nice, if the description of the corresponding sysctls in 
Documentation/networking/ip-sysctl.txt could be updated too?

Cheers
Carsten

^ permalink raw reply

* Re: [PATCH 2/2] ipvs: Extend MTU check to account for IPv6 NAT defrag changes
From: Simon Horman @ 2012-08-28  8:28 UTC (permalink / raw)
  To: Patrick McHardy
  Cc: Julian Anastasov, Jesper Dangaard Brouer, netdev, lvs-devel,
	Pablo Neira Ayuso, Hans Schillstrom, Wensong Zhang,
	netfilter-devel
In-Reply-To: <Pine.GSO.4.63.1208281020530.18770@stinky-local.trash.net>

On Tue, Aug 28, 2012 at 10:22:05AM +0200, Patrick McHardy wrote:
> On Mon, 27 Aug 2012, Julian Anastasov wrote:
> 
> >
> >	Hello,
> >
> >On Mon, 27 Aug 2012, Jesper Dangaard Brouer wrote:
> >
> >>This patch is necessary, to make IPVS work, after Patrick McHardys
> >>IPv6 NAT defragmentation changes.
> >>
> >>Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com>
> >>---
> >>
> >>I would appriciate, if someone (e.g. Julian) double check the tunnel mode code.
> >
> >	Sure
> >
> >>--Jesper
> >>
> >> net/netfilter/ipvs/ip_vs_xmit.c |   16 +++++++++++++---
> >> 1 files changed, 13 insertions(+), 3 deletions(-)
> >>
> >>@@ -956,8 +963,11 @@ ip_vs_tunnel_xmit_v6(struct sk_buff *skb, struct ip_vs_conn *cp,
> >> 		skb_dst(skb)->ops->update_pmtu(skb_dst(skb), NULL, skb, mtu);
> >>
> >> 	/* MTU checking: Special for tunnel mode */
> >>-	if (mtu < ntohs(old_iph->payload_len) + sizeof(struct ipv6hdr) &&
> >>-	    !skb_is_gso(skb)) {
> >>+	if ((!IP6CB(skb)->frag_max_size &&
> >>+	     (mtu < ntohs(old_iph->payload_len) + sizeof(struct ipv6hdr) &&
> >>+	      !skb_is_gso(skb)))
> >>+	    || IP6CB(skb)->frag_max_size + sizeof(struct ipv6hdr) > mtu) {
> >>+
> >
> >	mtu is already reduced with the new outer header size,
> >may be we can just call __mtu_check_toobig_v6 with mtu?
> >
> >> 		if (!skb->dev) {
> >> 			struct net *net = dev_net(skb_dst(skb)->dev);
> >
> >	All other changes in patch 1 and 2 look ok and
> >I'll ack them next time.
> 
> I'd like to take the final patches through my nat-ipv6 tree in order
> to avoid temporary regressions. Please let me know when they're ready
> for applying.

I'm fine with these changes going through your tree (rather than my IPVS tree)
once they are ready.

^ permalink raw reply

* Re: [PATCH 2/2] ipvs: Extend MTU check to account for IPv6 NAT defrag changes
From: Patrick McHardy @ 2012-08-28  8:22 UTC (permalink / raw)
  To: Julian Anastasov
  Cc: Jesper Dangaard Brouer, netdev, lvs-devel, Simon Horman,
	Pablo Neira Ayuso, Hans Schillstrom, Wensong Zhang,
	netfilter-devel
In-Reply-To: <alpine.LFD.2.00.1208271810070.24809@ja.ssi.bg>

On Mon, 27 Aug 2012, Julian Anastasov wrote:

>
> 	Hello,
>
> On Mon, 27 Aug 2012, Jesper Dangaard Brouer wrote:
>
>> This patch is necessary, to make IPVS work, after Patrick McHardys
>> IPv6 NAT defragmentation changes.
>>
>> Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com>
>> ---
>>
>> I would appriciate, if someone (e.g. Julian) double check the tunnel mode code.
>
> 	Sure
>
>> --Jesper
>>
>>  net/netfilter/ipvs/ip_vs_xmit.c |   16 +++++++++++++---
>>  1 files changed, 13 insertions(+), 3 deletions(-)
>>
>> @@ -956,8 +963,11 @@ ip_vs_tunnel_xmit_v6(struct sk_buff *skb, struct ip_vs_conn *cp,
>>  		skb_dst(skb)->ops->update_pmtu(skb_dst(skb), NULL, skb, mtu);
>>
>>  	/* MTU checking: Special for tunnel mode */
>> -	if (mtu < ntohs(old_iph->payload_len) + sizeof(struct ipv6hdr) &&
>> -	    !skb_is_gso(skb)) {
>> +	if ((!IP6CB(skb)->frag_max_size &&
>> +	     (mtu < ntohs(old_iph->payload_len) + sizeof(struct ipv6hdr) &&
>> +	      !skb_is_gso(skb)))
>> +	    || IP6CB(skb)->frag_max_size + sizeof(struct ipv6hdr) > mtu) {
>> +
>
> 	mtu is already reduced with the new outer header size,
> may be we can just call __mtu_check_toobig_v6 with mtu?
>
>>  		if (!skb->dev) {
>>  			struct net *net = dev_net(skb_dst(skb)->dev);
>
> 	All other changes in patch 1 and 2 look ok and
> I'll ack them next time.

I'd like to take the final patches through my nat-ipv6 tree in order
to avoid temporary regressions. Please let me know when they're ready
for applying.

^ permalink raw reply

* IPv4 header extension for IPv6 translation support
From: Vlad Maraev @ 2012-08-28  7:59 UTC (permalink / raw)
  To: netdev
  Cc: Влад Мараев

Hi,
I would be happy to get some feedback on my idea that is described below.
I need to tell that I'm a newbie in Linux kernel development but it
would be interesting for me to estimate complexity and to implement
this in practice.

Best Regards,
Vlad Maraev
State University of Telecommunications
St. Petersburg, Russia

-------------------------------------------------------

Why?
----------
During the process of transitioning to IPv6 segments that use
different versions of IP need to interconnect. On the ISP level it can
only be done with NATs. I mean only 6-4 or 4-6 connections, not 4-6-4
or 6-4-6 that can be achieved with tunnel or dual stack in the middle.

NATs have two major disadvantages: scalability and latency. NAT that
is connecting large network segments needs to maintain a huge
translation table. Search in such tables requires lots of CPU/memory
resources.

The protocol
----------
IPv4x (IPv4 eXtended) aims to solve this problem. I think the major
problem with IPv4 is that the IPv4 packet doesn't contain IPv6
address. If it would contain IPv6 address, the 4-6 interaction can be
provided by stateless algorithmic protocol translation without any NAT
tables.

IPv4x header inherits its structure from IPv4 with 3 additional 32-bit
fields after the Destination Address. This field will be used for
extending source or destination address to IPv6 format. IHL field is
also changed.

IPv6 address will be divided in two parts: 32 and 96 bits. First part
will be included in Source or Destination Address field (depending on
the frame direction). Remaining 96 bits will be placed after the
Destination Address.

The assignable Global Unicast Address space is defined
in as being the address block defined by the prefix 2000::/3. First
bits of this addresses are 0010 that means the network of A class in
IPv4 space that could lead to correlation with existing addresses. To
solve this first character is changed from `2` to `F`. First bits of
`F` (1111) will form IPv4 address of class E that is reserved and
could be defined for IPv6 transition. E-class

Example:
-------------------------
| 2001:0db8:cafe:3:2::2 |
-------------------------
      v
-------------
| F001:0db8 | (Destination address)
-------------
| cafe:0003 | (96 bits below)
| 0000:0000 |
| 0000:0002 |
-------------
	  v
----------------
| 240.1.13.184 | (Destination address - IPv4 format)
----------------
| cafe:0003    | (96 bits below)
| 0000:0000    |
| 0000:0002    |
----------------

Transmission of IPv4x packet
----------
IPv4 transmission of IPv4x packet can be processed using IPv4 routing.
Intermediate nodes don't need to implement IPv4x, hey will work with
IPv4x packet as a normal IPv4 packet. Border nodes must implement
IPv4x and provide algorithmic translation of IPv4x frame to IPv6.


Example:
-------------
| F001:0db8 | (Destination address)
-------------
| cafe:0003 | (96 bits below)
| 0000:0000	|
| 0000:0002 |
-------------	
	 v
-------------------------
| 64:ff9b::203.0.113.1  |  source address can be written using some prefix
-------------------------
| 2001:0db8:cafe:3:2::2 |  destination IPv6 address
-------------------------

Problems
----------
1. Intermediate nodes can look below the Destination Address for the
options if ihl>5 and will be confused with IPv4x data.
2. All the IPv4x packets would be routed to one node. But there can be
several different organization IPv6 networks connected to this node.

First steps
----------
1. IPv4x in linux kernel
2. ICMP with ping4x
3. Transmission testing. IPv4x ping request must be routed to node
with class E interface.

^ permalink raw reply

* [PATCH] iproute2: tc.8: update UNITS section.
From: Li Wei @ 2012-08-28  5:38 UTC (permalink / raw)
  To: shemminger; +Cc: netdev, Li Wei

- rename section UNITS to PARAMETERS.
- break section PARAMETERS down to four subsections to cover the
  common used parameter types(RATES, TIMES, SIZES, VALUES).
- add some explaination for IEC units in RATES.
- point out the max value we can set for RATES, TIMES and SIZES.

Signed-off-by: Li Wei <lw@cn.fujitsu.com>
---
 man/man8/tc.8 |  111 +++++++++++++++++++++++++++++++++++++++++++--------------
 1 file changed, 85 insertions(+), 26 deletions(-)

diff --git a/man/man8/tc.8 b/man/man8/tc.8
index 98fbfcd..44d644a 100644
--- a/man/man8/tc.8
+++ b/man/man8/tc.8
@@ -257,19 +257,20 @@ qdiscs applies.
 FILTERS
 Filters have a three part ID, which is only needed when using a hashed
 filter hierarchy.
-.SH UNITS
-All parameters accept a floating point number, possibly followed by a unit.
-.P
-Bandwidths or rates can be specified in:
-.TP
-bps
-Bytes per second
+
+.SH PARAMETERS
+These following parameters are widely used in TC, for other parameters,
+see the man page for individual qdiscs.
+
 .TP
-kbps
-Kilobytes per second
+RATES
+Bandwidths or rates.
+These parameters accept a floating point number, possibly followed by
+a unit(both SI and IEC units supported).
+.RS
 .TP
-mbps
-Megabytes per second
+bit or a bare number
+Bits per second
 .TP
 kbit
 Kilobits per second
@@ -277,27 +278,41 @@ Kilobits per second
 mbit
 Megabits per second
 .TP
-bit or a bare number
-Bits per second
-.P
-Amounts of data can be specified in:
+gbit
+Gegabits per second
 .TP
-kb or k
-Kilobytes
+tbit
+Terabits per second
 .TP
-mb or m
-Megabytes
+bps
+Bytes per second
 .TP
-mbit
-Megabits
+kbps
+Kilobytes per second
 .TP
-kbit
-Kilobits
+mbps
+Megabytes per second
 .TP
-b or a bare number
-Bytes.
+gbps
+Gigabytes per second
+.TP
+tbps
+Terabytes per second
+
+.P
+To specified in IEC units, just replace the SI prefix(k-, m-, g-, t-) with
+IEC prefix(ki-, mi-, gi- and ti-) respectively.
+
 .P
-Lengths of time can be specified in:
+TC store rates as a __u32 integer in bps internally,
+so we can specify a max rate of UINT32_MAX bps.
+.RE
+
+.TP
+TIMES
+Length of time. Can be specified as a float pointing number 
+followed by an optional unit:
+.RS
 .TP
 s, sec or secs
 Whole seconds
@@ -308,6 +323,50 @@ Milliseconds
 us, usec, usecs or a bare number
 Microseconds.
 
+.P
+TC defined it's own time unit(equals to microsecond) and store time values
+as __u32 integer, thus we can specify a max time value of UINT32_MAX usecs.
+.RE
+
+.TP
+SIZES
+Amounts of data. Can be specified as a float pointing number 
+followed by an optional unit:
+.RS
+.TP
+b or a bare number
+Bytes.
+.TP
+kbit
+Kilobites
+.TP
+kb or k
+Kilobytes
+.TP
+mbit
+Megabits
+.TP
+mb or m
+Megabytes
+.TP
+gbit
+Gigabites
+.TP
+gb or g
+Gigabytes
+
+.P
+TC store sizes internally as __u32 integer in byte, so we can specify
+a max size of UINT32_MAX bytes.
+.RE
+
+.TP
+VALUES
+Other values without a unit.
+These parameters read as decimal by default, but you can
+indicate TC to read them as octal and hexadecimal by adding a '0'
+or '0x' prefix respectively.
+
 .SH TC COMMANDS
 The following commands are available for qdiscs, classes and filter:
 .TP
-- 
1.7.10.1

^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox