* [PATCH 2/2] bonding: COW before overwriting the destination MAC address
From: Changli Gao @ 2011-03-03 4:25 UTC (permalink / raw)
To: Jay Vosburgh; +Cc: David S. Miller, netdev, Changli Gao
When there is a ptype handler holding a clone of this skb, whose
destination MAC addresse is overwritten, the owner of this handler may
get a corrupted packet.
Signed-off-by: Changli Gao <xiaosuo@gmail.com>
---
drivers/net/bonding/bond_main.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index 912b416..211be52 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -1513,6 +1513,11 @@ static struct sk_buff *bond_handle_frame(struct sk_buff *skb)
skb->pkt_type == PACKET_HOST) {
u16 *dest = (u16 *) eth_hdr(skb)->h_dest;
+ if (unlikely(skb_cow_head(skb,
+ skb->data - skb_mac_header(skb)))) {
+ kfree_skb(skb);
+ return NULL;
+ }
memcpy(dest, bond_dev->dev_addr, ETH_ALEN);
}
^ permalink raw reply related
* [PATCH 1/2] bonding: remove the unused dummy functions when net poll controller isn't enabled
From: Changli Gao @ 2011-03-03 4:26 UTC (permalink / raw)
To: Jay Vosburgh; +Cc: David S. Miller, netdev, Changli Gao
These two functions are only used when net poll controller is enabled.
Signed-off-by: Changli Gao <xiaosuo@gmail.com>
---
drivers/net/bonding/bond_main.c | 8 --------
1 file changed, 8 deletions(-)
diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index 0592e6d..912b416 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -1380,14 +1380,6 @@ static inline void slave_disable_netpoll(struct slave *slave)
static void bond_netpoll_cleanup(struct net_device *bond_dev)
{
}
-static int bond_netpoll_setup(struct net_device *dev, struct netpoll_info *ni)
-{
- return 0;
-}
-static struct netpoll_info *bond_netpoll_info(struct bonding *bond)
-{
- return NULL;
-}
#endif
/*---------------------------------- IOCTL ----------------------------------*/
^ permalink raw reply related
* inetpeer with create==0
From: David Miller @ 2011-03-03 4:45 UTC (permalink / raw)
To: eric.dumazet; +Cc: netdev
Eric, I was profiling the non-routing-cache case and something that stuck
out is the case of calling inet_getpeer() with create==0.
If an entry is not found, we have to redo the lookup under a spinlock
to make certain that a concurrent writer rebalancing the tree does
not "hide" an existing entry from us.
This makes the case of a create==0 lookup for a not-present entry
really expensive. It is on the order of 600 cpu cycles on my
Niagara2.
I added a hack to not do the relookup under the lock when create==0
and it now costs less than 300 cycles.
This is now a pretty common operation with the way we handle COW'd
metrics, so I think it's definitely worth optimizing.
I looked at the generic radix tree implementation, and it supports
full RCU lookups in parallel with insert/delete. It handles the race
case without the relookup under lock because it creates fixed paths
to "slots" where nodes live using shifts and masks. So if a path
to a slot ever existed, it will always exist.
Take a look at lib/radix-tree.c and include/linux/radix-tree.h if
you are curious.
I think we should do something similar for inetpeer. Currently we
cannot just use the existing generic radix-tree code because it only
supports indexes as large as "unsigned long" and we need to handle
128-bit ipv6 addresses.
^ permalink raw reply
* Re: [PATCH 1/5] tg3: use usleep_range not msleep for small sleeps
From: David Miller @ 2011-03-03 4:55 UTC (permalink / raw)
To: martinez.javier; +Cc: mcarlson, mchan, grant.likely, netdev, kernelnewbies
In-Reply-To: <1298999069-12740-2-git-send-email-martinez.javier@gmail.com>
From: Javier Martinez Canillas <martinez.javier@gmail.com>
Date: Tue, 1 Mar 2011 18:04:26 +0100
>
> Signed-off-by: Javier Martinez Canillas <martinez.javier@gmail.com>
Why?
^ permalink raw reply
* Re: [PATCH 3/5] tg3: Enclose macro with complex values in parenthesis
From: David Miller @ 2011-03-03 4:56 UTC (permalink / raw)
To: martinez.javier; +Cc: mcarlson, mchan, grant.likely, netdev, kernelnewbies
In-Reply-To: <1298999069-12740-3-git-send-email-martinez.javier@gmail.com>
From: Javier Martinez Canillas <martinez.javier@gmail.com>
Date: Tue, 1 Mar 2011 18:04:27 +0100
> Macros with complex values should be enclosed in parenthesis.
> Change accordingly.
>
> Signed-off-by: Javier Martinez Canillas <martinez.javier@gmail.com>
This seems unnecessary, what problem do the current macro definitions
create?
^ permalink raw reply
* Re: inetpeer with create==0
From: Changli Gao @ 2011-03-03 5:30 UTC (permalink / raw)
To: David Miller; +Cc: eric.dumazet, netdev
In-Reply-To: <20110302.204545.193730647.davem@davemloft.net>
On Thu, Mar 3, 2011 at 12:45 PM, David Miller <davem@davemloft.net> wrote:
>
>
> I looked at the generic radix tree implementation, and it supports
> full RCU lookups in parallel with insert/delete. It handles the race
> case without the relookup under lock because it creates fixed paths
> to "slots" where nodes live using shifts and masks. So if a path
> to a slot ever existed, it will always exist.
>
> Take a look at lib/radix-tree.c and include/linux/radix-tree.h if
> you are curious.
>
> I think we should do something similar for inetpeer. Currently we
> cannot just use the existing generic radix-tree code because it only
> supports indexes as large as "unsigned long" and we need to handle
> 128-bit ipv6 addresses.
I am just wondering why we need a trie(radix tree) here, we don't have
to do LPM since the lengths of the keys are always fixed(ether 4 or
16). Maybe a hash table is enough and simpler, and it is RCU
compatible.
--
Regards,
Changli Gao(xiaosuo@gmail.com)
^ permalink raw reply
* Re: inetpeer with create==0
From: David Miller @ 2011-03-03 5:36 UTC (permalink / raw)
To: xiaosuo; +Cc: eric.dumazet, netdev
In-Reply-To: <AANLkTimwaUaC7Xu2djSNR9k98sYbSM0V88sCAc34R6Ac@mail.gmail.com>
From: Changli Gao <xiaosuo@gmail.com>
Date: Thu, 3 Mar 2011 13:30:03 +0800
> I am just wondering why we need a trie(radix tree) here, we don't have
> to do LPM since the lengths of the keys are always fixed(ether 4 or
> 16). Maybe a hash table is enough and simpler, and it is RCU
> compatible.
Because trie eliminates all of the issues of having to size a hash
table, dynamically resize it, etc.
Trie gives well bounded performance dependent solely upon size of
the table, rather than access patterns, distribution of keys, and
how perfect hash function is.
We used to use a hash table for the page cache too, but now we use
a trie structure there as well, and that uses full long word sized
keys and the generic raidx-tree code.
Using hash tables is really foolish for potentially large data sets.
^ permalink raw reply
* bonding...
From: David Miller @ 2011-03-03 5:49 UTC (permalink / raw)
To: fubar; +Cc: netdev
Hey, if someone could step up and help with bonding maintainence
in some tangible way, I'd really appreciate it.
Currently the situation is that many people work on bonding patches,
and whilst I do try and wait for some ACKs to arrive, I am the person
who has to sort out when changes are ready, decide to apply them, and
poke for review when things fall through the cracks.
Sometimes patches go for weeks without ACKs, and in that situation
I have to either try to understand the changes myself, or wait
potentially forever for someone with bonding knowledge to take a
good look at the patch and properly review it.
It was nearly 2 weeks before Oleg V. Ukhno's 802.3ad round-robin patch
got looked at by anyone with bonding knowledge. And it only happened
because I got tired of seeing his poor patch rot in patchwork
and had to explicitly asked for review the other day.
This is unacceptable, people are submitting multiple bonding patches
every single day now. It needs a clueful bonding person looking at
these submissions on a constant basis.
This is a serious problem and is backlogging the netdev patch queue.
So if someone would become an active bonding patch-accumulator, and
send me sets of patches that are ready to apply, I would really
appreciate it.
Thanks.
^ permalink raw reply
* r8169 disable ASPM patch
From: David Miller @ 2011-03-03 5:52 UTC (permalink / raw)
To: romieu; +Cc: netdev
What is the status of Stanislaw Gruszka's "disable ASPM" r8169
patch?
http://patchwork.ozlabs.org/patch/83961/
Is it waiting for your testing? Is it waiting for feedback from
Hayes?
Is there going to be a MAINTAINERS patch that adds Hayes? If so
what is stalling that?
Thanks.
^ permalink raw reply
* Re: [PATCH] Bluetooth: Fix BT_L2CAP and BT_SCO in Kconfig
From: David Miller @ 2011-03-03 5:54 UTC (permalink / raw)
To: padovan-Y3ZbgMPKUGA34EUeqzHoZw
Cc: linville-2XuSBdqkA4R54TAoqtyWWQ,
linux-bluetooth-u79uwXL29TY76Z2rM5mHXA,
netdev-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1298684485-3081-1-git-send-email-padovan-Y3ZbgMPKUGA34EUeqzHoZw@public.gmane.org>
From: "Gustavo F. Padovan" <padovan-Y3ZbgMPKUGA34EUeqzHoZw@public.gmane.org>
Date: Fri, 25 Feb 2011 22:41:25 -0300
> If we want something "bool" built-in in something "tristate" it can't
> "depend on" the tristate config option.
>
> Report by DaveM:
>
> I give it 'y' just to make it happen, for both, and afterways no
> matter how many times I rerun "make oldconfig" I keep seeing things
> like this in my build:
>
> scripts/kconfig/conf --silentoldconfig Kconfig
> include/config/auto.conf:986:warning: symbol value 'm' invalid for BT_SCO
> include/config/auto.conf:3156:warning: symbol value 'm' invalid for BT_L2CAP
>
> Reported-by: David S. Miller <davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org>
> Signed-off-by: Gustavo F. Padovan <padovan-Y3ZbgMPKUGA34EUeqzHoZw@public.gmane.org>
I think this approach is fine since it mirrors what we use in other similar
situations.
I am assuming this patch will propagate via bluetooth --> wireless --> me.
Thanks.
^ permalink raw reply
* Re: [PATCH nex-next] netdevice: make initial group visible to userspace
From: David Miller @ 2011-03-03 5:56 UTC (permalink / raw)
To: ddvlad; +Cc: netdev, shemminger, kaber
In-Reply-To: <20110227083911.GA29638@cormyr>
From: Vlad Dogaru <ddvlad@rosedu.org>
Date: Sun, 27 Feb 2011 10:39:12 +0200
> [PATCH net-next] netdevice: make initial group visible to userspace
>
> INIT_NETDEV_GROUP is needed by userspace, move it outside __KERNEL__
> guards.
>
> Signed-off-by: Vlad Dogaru <ddvlad@rosedu.org>
Applied, thanks.
^ permalink raw reply
* Re: [PATCH net-next-2.6 v2 1/2] dcbnl: add support for retrieving peer configuration - ieee
From: David Miller @ 2011-03-03 5:59 UTC (permalink / raw)
To: shmulikr; +Cc: john.r.fastabend, eilong, netdev
In-Reply-To: <1298819071.4544.9.camel@lb-tlvb-shmulik.il.broadcom.com>
From: "Shmulik Ravid" <shmulikr@broadcom.com>
Date: Sun, 27 Feb 2011 17:04:31 +0200
> These 2 patches add the support for retrieving the remote or peer DCBX
> configuration via dcbnl for embedded DCBX stacks. The peer configuration
> is part of the DCBX MIB and is useful for debugging and diagnostics of
> the overall DCB configuration. The first patch add this support for IEEE
> 802.1Qaz standard the second patch add the same support for the older
> CEE standard. Diff for v2 - the peer-app-info is CEE specific.
>
>
> Signed-off-by: Shmulik Ravid <shmulikr@broadcom.com>
Applied.
^ permalink raw reply
* [RFC] Keep track of interrupt URB status and resubmit in bh if necessary
From: Paul Stewart @ 2011-03-03 5:45 UTC (permalink / raw)
To: netdev; +Cc: dbrownell, Indrek.Peri
It appears that a patch from Indrek Peri similar to the one below
without resolution. I'm new to this problem (suspend-resume causing
interrupt URBs to stop delivering) and am curious about what the correct
solution would should like. Before becoming aware of this thread, I
just added a "usb_submit_urb" of "dev->interrupt" into "usbnet_resume()"
and that worked to solve the issue I was having. Apparently this isn't
the correct solution though, from David's response to Indrek. So, I'm
curious about what the right code should be.
I'll note is that submitting the interrupt URB seems fairly benign. If
we are in a situation where we should not have sent an URB (e.g, the
netif wasn't running) intr_complete correctly handles this case and does
not re-submit the URB, so at most we get one "rogue" interrupt URB after
resume-from-suspend. The only nasty thing is that this URB should
probably not be submitted from interrupt, which the resume function
almost certainly is. I'm guessing this is part of why David NAKed
Indrek's patch. Am I correct?
Does something like the patch below seem like a resonable solution?
Cc: David Brownell <dbrownell@users.sourceforge.org>
Cc: Indrek Peri <Indrek.Peri@Ericsson.com>
Signed-off-by: Paul Stewart <pstew@google.com>
---
drivers/net/usb/usbnet.c | 21 +++++++++++++++++++--
include/linux/usb/usbnet.h | 1 +
2 files changed, 20 insertions(+), 2 deletions(-)
diff --git a/drivers/net/usb/usbnet.c b/drivers/net/usb/usbnet.c
index 02d25c7..bc6a8e0 100644
--- a/drivers/net/usb/usbnet.c
+++ b/drivers/net/usb/usbnet.c
@@ -471,6 +471,8 @@ static void intr_complete (struct urb *urb)
struct usbnet *dev = urb->context;
int status = urb->status;
+ dev->interrupt_urb_running = 0;
+
switch (status) {
/* success */
case 0:
@@ -497,7 +499,9 @@ static void intr_complete (struct urb *urb)
memset(urb->transfer_buffer, 0, urb->transfer_buffer_length);
status = usb_submit_urb (urb, GFP_ATOMIC);
- if (status != 0 && netif_msg_timer (dev))
+ if (status == 0)
+ dev->interrupt_urb_running = 1;
+ else if (netif_msg_timer (dev))
deverr(dev, "intr resubmit --> %d", status);
}
@@ -580,6 +584,7 @@ static int usbnet_stop (struct net_device *net)
remove_wait_queue (&unlink_wakeup, &wait);
usb_kill_urb(dev->interrupt);
+ dev->interrupt_urb_running = 0;
/* deferred work (task, timer, softirq) must also stop.
* can't flush_scheduled_work() until we drop rtnl (later),
@@ -640,7 +645,8 @@ static int usbnet_open (struct net_device *net)
if (netif_msg_ifup (dev))
deverr (dev, "intr submit %d", retval);
goto done;
- }
+ } else
+ dev->interrupt_urb_running = 1;
}
netif_start_queue (net);
@@ -1065,6 +1071,17 @@ static void usbnet_bh (unsigned long param)
if (dev->txq.qlen < TX_QLEN (dev))
netif_wake_queue (dev->net);
}
+
+ // Do we need to re-enable interrupt URBs?
+ if (netif_running (dev->net) &&
+ netif_device_present (dev->net) &&
+ dev->interrupt_urb_running == 0) {
+ usb_submit_urb (dev->interrupt, GFP_KERNEL);
+
+ /* Unconditionally mark as running so we don't retry */
+ dev->interrupt_urb_running = 1;
+ }
+
}
diff --git a/include/linux/usb/usbnet.h b/include/linux/usb/usbnet.h
index ba09fe8..1b8ed8a 100644
--- a/include/linux/usb/usbnet.h
+++ b/include/linux/usb/usbnet.h
@@ -49,6 +49,7 @@ struct usbnet {
u32 hard_mtu; /* count any extra framing */
size_t rx_urb_size; /* size for rx urbs */
struct mii_if_info mii;
+ int interrupt_urb_running;
/* various kinds of pending driver work */
struct sk_buff_head rxq;
--
1.7.3.1
^ permalink raw reply related
* Re: [PATCH net-next-2.6 v2 2/2] dcbnl: add support for retrieving peer configuration - cee
From: David Miller @ 2011-03-03 5:59 UTC (permalink / raw)
To: shmulikr; +Cc: john.r.fastabend, eilong, netdev
In-Reply-To: <1298819078.4544.10.camel@lb-tlvb-shmulik.il.broadcom.com>
From: "Shmulik Ravid" <shmulikr@broadcom.com>
Date: Sun, 27 Feb 2011 17:04:38 +0200
> This patch adds the support for retrieving the remote or peer DCBX
> configuration via dcbnl for embedded DCBX stacks supporting the CEE DCBX
> standard.
>
> Signed-off-by: Shmulik Ravid <shmulikr@broadcom.com>
Applied.
^ permalink raw reply
* Re: [patch net-next-2.6] bonding: remove skb_share_check in handle_frame
From: Jiri Pirko @ 2011-03-03 6:14 UTC (permalink / raw)
To: Nicolas de Pesloüan
Cc: Andy Gospodarek, netdev, davem, fubar, eric.dumazet
In-Reply-To: <4D6EBC7B.8040502@gmail.com>
Wed, Mar 02, 2011 at 10:54:03PM CET, nicolas.2p.debian@gmail.com wrote:
>Le 02/03/2011 22:12, Jiri Pirko a écrit :
>>Wed, Mar 02, 2011 at 09:47:50PM CET, nicolas.2p.debian@gmail.com wrote:
>>>Hi Jiri,
>>>
>>>Do you plan to call the bonding ARP handler from inside bond_handle_frame()?
>>
>>I do - it's part of patchset I've cooked (going to test that tomorrow).
>>
>>>A few days ago
>>>(http://marc.info/?l=linux-netdev&m=129883949022340&w=2), I noticed
>>>that it is not possible to call the bonding ARP handler from inside
>>>the bonding rx_handler, because some frame processing may be required
>>>after the bonding rx_handler call, to put the frame in a suitable
>>>state for the bonding ARP handler.
>>
>>Do you see another scenario besides the next one?
>
>None that currently work, but eth0 -> bond0 -> br0 -> br0.100 should work too.
>
>>>This is at least true with the following setup, eth0 -> bond0 ->
>>>bond0.100, where the ARP frames are VLAN tagged at the time the
>>>bonding rx_handler process them.
>>
>>Isn't this scenario resolved by vlan_on_bond_hook ?
>>
>>eth0
>> ->rx_handler -> another round
>>bond0
>> ->vlan_hwaccel_do_receive -> __netif_receive_skb
>>bond0.100
>> ->vlan_on_bond_hook -> reinject to bond0
>
>Yes, it is, but this hack does not solve the eth0 -> bond0 -> br0 -> br0.100 configuration.
>
>All those handlers that call netif_rx() or __netif_receive_skb()
>sound horrible to me. Can you imagine the global overhead of the
>above receive path?
>
>The reason why I suggested you introduce the goto another_round is
>because most - if not all - stacking configurations could/should be
>handled simply by returning the right skb from the rx_handler and let
>__netif_receive_skb() loop. And by having the right orig_dev logic
>inside __netif_receive_skb(), it could be possible to remove the
>current vlan_on_bond_hook hack.
Well that wouldn't solve the problem. if we just got anorther_round the
packed would not be delivered to bond0.100 but only to bond0. That's
also unwanted. Well, this think shouldn't have been added here in the
first place :(
>
>My question about whether the skb is shared between the protocol
>handlers (in another thread) also target at this idea.
>
>You will probably told me I'm free to propose patchs for all that,
>and you are right. Just missing the time to do so.
>
> Nicolas.
^ permalink raw reply
* Re: [Patch] bonding: move procfs into bond_procfs.c
From: David Miller @ 2011-03-03 6:17 UTC (permalink / raw)
To: amwang; +Cc: linux-kernel, fubar, netdev
In-Reply-To: <1298896204-6521-1-git-send-email-amwang@redhat.com>
From: Amerigo Wang <amwang@redhat.com>
Date: Mon, 28 Feb 2011 20:30:04 +0800
> bond_main.c is bloating, separate the procfs code out,
> move them to bond_procfs.c
>
> Signed-off-by: WANG Cong <amwang@redhat.com>
Please use Makefile rules to conditionally compile the new bond_procfs.o
file, instead of wrapping almost the entire file with an ifdef.
Put the dummy no-PROC_FS function stubs, as inlines, in bonding.h
Thanks!
^ permalink raw reply
* Re: [PATCH] RxRPC: Fix v1 keys
From: David Miller @ 2011-03-03 6:19 UTC (permalink / raw)
To: dhowells; +Cc: netdev, linux-kernel, anton
In-Reply-To: <20110228132753.30463.24909.stgit@warthog.procyon.org.uk>
From: David Howells <dhowells@redhat.com>
Date: Mon, 28 Feb 2011 13:27:53 +0000
> From: Anton Blanchard <anton@au1.ibm.com>
>
> commit 339412841d7 (RxRPC: Allow key payloads to be passed in XDR form)
> broke klog for me. I notice the v1 key struct had a kif_version field
> added:
>
> -struct rxkad_key {
> - u16 security_index; /* RxRPC header security index */
> - u16 ticket_len; /* length of ticket[] */
> - u32 expiry; /* time at which expires */
> - u32 kvno; /* key version number */
> - u8 session_key[8]; /* DES session key */
> - u8 ticket[0]; /* the encrypted ticket */
> -};
>
> +struct rxrpc_key_data_v1 {
> + u32 kif_version; /* 1 */
> + u16 security_index;
> + u16 ticket_length;
> + u32 expiry; /* time_t */
> + u32 kvno;
> + u8 session_key[8];
> + u8 ticket[0];
> +};
>
> However the code in rxrpc_instantiate strips it away:
>
> data += sizeof(kver);
> datalen -= sizeof(kver);
>
> Removing kif_version fixes my problem.
>
> Signed-off-by: Anton Blanchard <anton@samba.org>
> Signed-off-by: David Howells <dhowells@redhat.com>
Applied.
^ permalink raw reply
* Re: [PATCH] AF_RXRPC: Handle receiving ACKALL packets
From: David Miller @ 2011-03-03 6:19 UTC (permalink / raw)
To: dhowells; +Cc: netdev, linux-kernel
In-Reply-To: <20110228132743.30463.33897.stgit@warthog.procyon.org.uk>
From: David Howells <dhowells@redhat.com>
Date: Mon, 28 Feb 2011 13:27:43 +0000
> The OpenAFS server is now sending ACKALL packets, so we need to handle them.
> Otherwise we report a protocol error and abort.
>
> Signed-off-by: David Howells <dhowells@redhat.com>
Applied.
^ permalink raw reply
* Re: [PATCH] cxgb{3,4}*: improve Kconfig dependencies
From: David Miller @ 2011-03-03 6:23 UTC (permalink / raw)
To: dm; +Cc: netdev, linux-scsi, JBeulich
In-Reply-To: <1298950455-4497-1-git-send-email-dm@chelsio.com>
From: Dimitris Michailidis <dm@chelsio.com>
Date: Mon, 28 Feb 2011 19:34:15 -0800
> - Remove the dependency of cxgb4 and cxgb4vf on INET. cxgb3 really
> depends on INET, keep it but add it directly to the driver's Kconfig
> entry.
> - Make the iSCSI drivers cxgb3i and cxgb4i available in the SCSI menu
> without requiring any options in the net driver menu to be enabled
> first. Add needed selects so the iSCSI drivers can build their
> corresponding net drivers.
> - Remove CHELSIO_T*_DEPENDS.
>
> Signed-off-by: Dimitris Michailidis <dm@chelsio.com>
Applied to net-next-2.6, thanks!
^ permalink raw reply
* Re: inetpeer with create==0
From: Changli Gao @ 2011-03-03 6:27 UTC (permalink / raw)
To: David Miller; +Cc: eric.dumazet, netdev
In-Reply-To: <20110302.213634.189680857.davem@davemloft.net>
On Thu, Mar 3, 2011 at 1:36 PM, David Miller <davem@davemloft.net> wrote:
>
> Because trie eliminates all of the issues of having to size a hash
> table, dynamically resize it, etc.
>
> Trie gives well bounded performance dependent solely upon size of
> the table, rather than access patterns, distribution of keys, and
> how perfect hash function is.
Thanks for your explaination. Routing cache has all of these issues. :)
>
> We used to use a hash table for the page cache too, but now we use
> a trie structure there as well, and that uses full long word sized
> keys and the generic raidx-tree code.
>
> Using hash tables is really foolish for potentially large data sets.
>
However, I don't agree with you at every aspect.
Radix tree may cost lots of memory than a rbtree, avl tree or hash
table. Here is a case: turning to rbtree from radix tree.
http://git.kernel.org/?p=linux/kernel/git/davem/net-next-2.6.git;a=commit;h=8549164143a5431f9d9ea846acaa35a862410d9c
Since the keys of inetpeer rely on the access pattern, a attacker may
do a OOM DoS attack using sparse keys. We can't simply limit the
number of inetpeers to limit the memory cost of this subsystem. We
have to count the memory used for the inner branches of a radix tree.
Nevertheless, a attacker may also make this OS inefficiency by reduce
the max number of inetpeers.
Hash table + jhash have been proven a safe and efficient data
structure for large data sets(conntrack and ipvs), although the size
of the hash table may have to be adjusted by an administrator.
--
Regards,
Changli Gao(xiaosuo@gmail.com)
^ permalink raw reply
* RE: [Bugme-new] [Bug 29712] New: Bonding Driver(version : 3.5.0) - Problem with ARP monitoring in active backup mode
From: Harsha R02 @ 2011-03-03 6:31 UTC (permalink / raw)
To: Jay Vosburgh
Cc: Brian Haley, Andrew Morton, bugzilla-daemon, bugme-daemon, netdev
In-Reply-To: <17444.1298660550@death>
Hi Jay,
We found that the patch that is presented here has some issues and we
cannot go with this solution.
In function "bond_ab_arp_probe" in addition to sending arp probes for
the currently active slave we should also
be sending arp probes for the primary_slave if the link status of the
primary slave is up correct ?
I have made changes as below :
static void bond_ab_arp_probe(struct bonding *bond)
{
struct slave *slave;
int i;
read_lock(&bond->curr_slave_lock);
if (bond->current_arp_slave && bond->curr_active_slave)
pr_info(DRV_NAME "PROBE: c_arp %s && cas %s BAD\n",
bond->current_arp_slave->dev->name,
bond->curr_active_slave->dev->name);
if (bond->curr_active_slave) {
+ if((bond->curr_active_slave != bond->primary_slave) &&
+ (IS_UP(bond->primary_slave->dev))) {
+ bond_arp_send_all(bond, bond->primary_slave);
+ }
bond_arp_send_all(bond, bond->curr_active_slave);
read_unlock(&bond->curr_slave_lock);
return;
}
Please let us know if this can help us ? or if you see any side effects
?
Thanks,
Harsha
-----Original Message-----
From: Jay Vosburgh [mailto:fubar@us.ibm.com]
Sent: Saturday, February 26, 2011 12:33 AM
To: Harsha R02
Cc: Brian Haley; Andrew Morton; bugzilla-daemon@bugzilla.kernel.org;
bugme-daemon@bugzilla.kernel.org; netdev@vger.kernel.org
Subject: Re: [Bugme-new] [Bug 29712] New: Bonding Driver(version :
3.5.0) - Problem with ARP monitoring in active backup mode
Harsha R02 <Harsha.R02@mphasis.com> wrote:
>diff --git a/drivers/net/bonding/bond_main.c
b/drivers/net/bonding/bond_main.c
>index 40fb5ee..0413917 100644
>--- a/drivers/net/bonding/bond_main.c
>+++ b/drivers/net/bonding/bond_main.c
>@@ -3020,11 +3020,16 @@ static void bond_ab_arp_probe(struct bonding
*bond)
> bond->curr_active_slave->dev->name);
> if (bond->curr_active_slave) {
>+ if((bond->curr_active_slave != bond->primary_slave) &&
>+ (IS_UP(bond->primary_slave->dev)))
>+ goto failover;
>+
> bond_arp_send_all(bond, bond->curr_active_slave);
> read_unlock(&bond->curr_slave_lock);
> return;
> }
>+failover:
> read_unlock(&bond->curr_slave_lock);
> /* if we don't have a curr_active_slave, search for the next
available
I'm not sure this is the proper place to put the "failover:"
label, as it will go through the "search for any peer" logic that's
normally used when there are no available slaves. That will likely take
longer than simply switching to the primary.
It should be possible to simply call bond_change_active_slave
with the appropriate arguments; did you try this?
-J
>-----------------------------------------------------------------------
--------
>From: Harsha R02
>Sent: Fri 2/25/2011 6:14 PM
>To: Brian Haley; Andrew Morton
>Cc: bugzilla-daemon@bugzilla.kernel.org;
bugme-daemon@bugzilla.kernel.org;
>netdev@vger.kernel.org; Jay Vosburgh
>Subject: RE: [Bugme-new] [Bug 29712] New: Bonding Driver(version :
3.5.0) -
>Problem with ARP monitoring in active backup mode
>
>Attached patch resolves the issue. Failover happened back to primary
when it
>was up again in both the point to point and switch configuration.
>
>Please let us know if this change can be included.
>
>Thanks,
>
>- Harsha
>
>-----Original Message-----
>From: Brian Haley [mailto:brian.haley@hp.com]
>Sent: Friday, February 25, 2011 9:12 AM
>To: Andrew Morton
>Cc: Harsha R02; bugzilla-daemon@bugzilla.kernel.org;
>bugme-daemon@bugzilla.kernel.org; netdev@vger.kernel.org; Jay Vosburgh
>Subject: Re: [Bugme-new] [Bug 29712] New: Bonding Driver(version :
3.5.0) -
>Problem with ARP monitoring in active backup mode
>
>On 02/24/2011 05:51 PM, Andrew Morton wrote:
>> (switched to email. Please respond via emailed reply-to-all, not via
the
>> bugzilla web interface).
>>
>> On Wed, 23 Feb 2011 10:41:34 GMT
>> bugzilla-daemon@bugzilla.kernel.org wrote:
>>
>>> https://bugzilla.kernel.org/show_bug.cgi?id=29712
>>>
>>> Summary: Bonding Driver(version : 3.5.0) - Problem with
ARP
>>> monitoring in active backup mode
>>> Product: Drivers
>>> Version: 2.5
>>> Kernel Version: 2.6.32
>>
>> That's a paleolithic kernel you have there. This problem might have
>> been fixed already. Can you test a more recent kernel?
>
>I can add some more info since I originally looked at the problem.
This
>happens on 2.6.38 as well, and on this 2.6.32 kernel with a backported
>3.7.0 bonding driver (with the primary_reselect option). Harsha has a
>prototype patch that's being tested, but wanted to log the bug to see
>if one of the bonding maintainers had a better solution.
>
>I'll let him respond as I'm now out of the loop...
>
>Thanks,
>
>-Brian
---
-Jay Vosburgh, IBM Linux Technology Center, fubar@us.ibm.com
Information transmitted by this e-mail is proprietary to MphasiS, its associated companies and/ or its customers and is intended
for use only by the individual or entity to which it is addressed, and may contain information that is privileged, confidential or
exempt from disclosure under applicable law. If you are not the intended recipient or it appears that this mail has been forwarded
to you without proper authority, you are notified that any use or dissemination of this information in any manner is strictly
prohibited. In such cases, please notify us immediately at mailmaster@mphasis.com and delete this mail from your records.
^ permalink raw reply
* Re: inetpeer with create==0
From: David Miller @ 2011-03-03 6:42 UTC (permalink / raw)
To: xiaosuo; +Cc: eric.dumazet, netdev
In-Reply-To: <AANLkTi=Ty+C3b9U+151dUMpgGiwG3AE4dMmc3djWmp6v@mail.gmail.com>
From: Changli Gao <xiaosuo@gmail.com>
Date: Thu, 3 Mar 2011 14:27:40 +0800
> On Thu, Mar 3, 2011 at 1:36 PM, David Miller <davem@davemloft.net> wrote:
>>
>> Because trie eliminates all of the issues of having to size a hash
>> table, dynamically resize it, etc.
>>
>> Trie gives well bounded performance dependent solely upon size of
>> the table, rather than access patterns, distribution of keys, and
>> how perfect hash function is.
>
> Thanks for your explaination. Routing cache has all of these issues. :)
Thats why I am working hard to remove it :-)
The %99 percentile performance of our routing cache is absolutely
terrible. The routing cache does nothing except get in the way.
It is the reason why Linux is still completely unsuitable for use as a
router on the core internet.
> Radix tree may cost lots of memory than a rbtree, avl tree or hash
> table. Here is a case: turning to rbtree from radix tree.
> http://git.kernel.org/?p=linux/kernel/git/davem/net-next-2.6.git;a=commit;h=8549164143a5431f9d9ea846acaa35a862410d9c
I would not be against the use of rbtree or similar if it could be
done with with fully RCU non-locked lookups even in the not-present
case.
> Hash table + jhash have been proven a safe and efficient data
> structure for large data sets(conntrack and ipvs), although the size
> of the hash table may have to be adjusted by an administrator.
If anything is proven, it is that hashing based collections of cached
information are nothing but trouble when the contents are controlled in
some way by external entities.
Actually, back to the original topic, I wonder how bad it is to simply
elide the recheck in the create==0 case anyways. Except for the ipv4
fragmentation wraparound protection values, perfect inetpeer finding
is not necessary for correctness. And IPv4 fragmentation always calls
inetpeer with create!=0.
Eric?
^ permalink raw reply
* Re: [PATCH 2/2] bonding: COW before overwriting the destination MAC address
From: Eric Dumazet @ 2011-03-03 6:45 UTC (permalink / raw)
To: Changli Gao; +Cc: Jay Vosburgh, David S. Miller, netdev
In-Reply-To: <1299126321-9300-1-git-send-email-xiaosuo@gmail.com>
Le jeudi 03 mars 2011 à 12:25 +0800, Changli Gao a écrit :
> When there is a ptype handler holding a clone of this skb, whose
> destination MAC addresse is overwritten, the owner of this handler may
> get a corrupted packet.
>
> Signed-off-by: Changli Gao <xiaosuo@gmail.com>
> ---
> drivers/net/bonding/bond_main.c | 5 +++++
> 1 file changed, 5 insertions(+)
> diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
> index 912b416..211be52 100644
> --- a/drivers/net/bonding/bond_main.c
> +++ b/drivers/net/bonding/bond_main.c
> @@ -1513,6 +1513,11 @@ static struct sk_buff *bond_handle_frame(struct sk_buff *skb)
> skb->pkt_type == PACKET_HOST) {
> u16 *dest = (u16 *) eth_hdr(skb)->h_dest;
>
> + if (unlikely(skb_cow_head(skb,
> + skb->data - skb_mac_header(skb)))) {
> + kfree_skb(skb);
> + return NULL;
> + }
> memcpy(dest, bond_dev->dev_addr, ETH_ALEN);
> }
>
This seems buggy.
dest points to old skb head content.
^ permalink raw reply
* Re: inetpeer with create==0
From: Eric Dumazet @ 2011-03-03 6:51 UTC (permalink / raw)
To: David Miller; +Cc: netdev
In-Reply-To: <20110302.204545.193730647.davem@davemloft.net>
Le mercredi 02 mars 2011 à 20:45 -0800, David Miller a écrit :
> Eric, I was profiling the non-routing-cache case and something that stuck
> out is the case of calling inet_getpeer() with create==0.
>
> If an entry is not found, we have to redo the lookup under a spinlock
> to make certain that a concurrent writer rebalancing the tree does
> not "hide" an existing entry from us.
>
> This makes the case of a create==0 lookup for a not-present entry
> really expensive. It is on the order of 600 cpu cycles on my
> Niagara2.
>
Well, your test assumes all data is already on cpu caches ?
I'll take a look, but my reasoning was that the real cost in DDOS
situation is to bring data into caches. With a 20 depth, cache misses
costs are the problem.
The second lookup was basically free.
> I added a hack to not do the relookup under the lock when create==0
> and it now costs less than 300 cycles.
>
Hmm... I'm curious you send this hack to me ;)
> This is now a pretty common operation with the way we handle COW'd
> metrics, so I think it's definitely worth optimizing.
>
> I looked at the generic radix tree implementation, and it supports
> full RCU lookups in parallel with insert/delete. It handles the race
> case without the relookup under lock because it creates fixed paths
> to "slots" where nodes live using shifts and masks. So if a path
> to a slot ever existed, it will always exist.
>
> Take a look at lib/radix-tree.c and include/linux/radix-tree.h if
> you are curious.
>
> I think we should do something similar for inetpeer. Currently we
> cannot just use the existing generic radix-tree code because it only
> supports indexes as large as "unsigned long" and we need to handle
> 128-bit ipv6 addresses.
^ permalink raw reply
* Re: [GIT PULL nf-2.6] IPVS
From: Patrick McHardy @ 2011-03-03 6:53 UTC (permalink / raw)
To: Simon Horman
Cc: lvs-devel, netdev, netfilter-devel, netfilter, Hans Schillstrom,
Julian Anastasov
In-Reply-To: <20110302220637.GD8193@verge.net.au>
On 02.03.2011 23:06, Simon Horman wrote:
> On Wed, Mar 02, 2011 at 11:58:18AM +0100, Patrick McHardy wrote:
>> Am 01.03.2011 23:59, schrieb Simon Horman:
>>> Hi Patrick,
>>>
>>> please consider pulling
>>> git://git.kernel.org/pub/scm/linux/kernel/git/horms/lvs-test-2.6.git for-patrick
>>> to get the following change from Julian. Please note that it is an nf-2.6
>>> (that is 2.6.38-rc) change.
>>>
>>> Julian Anastasov (1):
>>> ipvs: fix dst_lock locking on dest update
>>
>> Pulled, thanks Simon.
>
> Thanks Patrick.
>
> This change is also needed in nf-next-2.6 but I assume that
> will automatically happen when nf-2.6 is merged into nf-next-2.6,
> possibly via a merge of net-2.6 into net-next-2.6.
Yes. If you need it as base for further work, the fastest way
is to ask Dave to pull net-2.6 into net-next-2.6, I can then
pull it into nf-next.
^ 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