* Re: [tcpdump-workers] Current wireless-testing breaks libpcap: mr_alen should be set
From: Jiri Pirko @ 2010-03-03 15:54 UTC (permalink / raw)
To: Frank W. Miller; +Cc: tcpdump-workers, linux-wireless, netdev
In-Reply-To: <A5B3F074CA554D3E95CFD349116E4D5B@FrankWMillerPC>
Wed, Mar 03, 2010 at 04:31:07PM CET, frankwmiller@frankwmiller.net wrote:
>
>Would this be preventing pcap_inject() from working say in kernel 2.6.31
>(stock FC12 kernel)?
Nope. The patch went in just recently.
>
>Thanks,
>FM
>
>
>> -----Original Message-----
>> From: tcpdump-workers-owner@lists.tcpdump.org [mailto:tcpdump-workers-
>> owner@lists.tcpdump.org] On Behalf Of Pavel Roskin
>> Sent: Tuesday, March 02, 2010 6:01 PM
>> To: linux-wireless@vger.kernel.org; netdev@vger.kernel.org; tcpdump-
>> workers@lists.tcpdump.org
>> Cc: Jiri Pirko
>> Subject: [tcpdump-workers] Current wireless-testing breaks libpcap:
>> mr_alen should be set
>>
>> Hello!
>>
>> The current wireless-testing appears to have some non-wireless bits from
>> the upcoming Linux 2.6.34. As a result, libpcap and all capture
>> programs that use it are broken.
>>
>> This patch to libpcap helps:
>>
>> --- a/pcap-linux.c
>> +++ b/pcap-linux.c
>> @@ -1563,6 +1563,7 @@ live_open_new(pcap_t *handle, const char
>> memset(&mr, 0, sizeof(mr));
>> mr.mr_ifindex = handle->md.ifindex;
>> mr.mr_type = PACKET_MR_PROMISC;
>> + mr.mr_alen = 6;
>> if (setsockopt(sock_fd, SOL_PACKET,
>> PACKET_ADD_MEMBERSHIP, &mr, sizeof(mr)) ==
>-1)
>> {
>>
>> libpcap git doesn't have the fix yet.
>>
>> The breakage must be coming from the commit 914c8ad2 by Jiri Pirko to
>> net/packet/af_packet.c
>>
>> I think it's very unhelpful to introduce patches that break significant
>> userspace functionality without giving the affected programs an advance
>> warning.
>>
>> Also, pulling bleeding edge stuff into wireless-testing before rc1
>> appears to be either a mistake or a bad decision.
>>
>> Sorry for cross-post, but it's an urgent issue. Repliers are encouraged
>> to trim the recipient list as necessary.
>>
>> --
>> Regards,
>> Pavel Roskin
>> -
>> This is the tcpdump-workers list.
>> Visit https://cod.sandelman.ca/ to unsubscribe.
>
^ permalink raw reply
* Re: [RFC][PATCH] ns: Syscalls for better namespace sharing control.
From: Serge E. Hallyn @ 2010-03-03 15:38 UTC (permalink / raw)
To: Eric W. Biederman
Cc: Sukadev Bhattiprolu, Pavel Emelyanov, Linux Netdev List,
containers, Netfilter Development Mailinglist, Ben Greear
In-Reply-To: <m1ocj6qljj.fsf@fess.ebiederm.org>
Quoting Eric W. Biederman (ebiederm@xmission.com):
> Sukadev Bhattiprolu <sukadev@linux.vnet.ibm.com> writes:
>
> > Eric W. Biederman [ebiederm@xmission.com] wrote:
> > |
> > | I think replacing a struct pid for another struct pid allocated in
> > | descendant pid_namespace (but has all of the same struct upid values
> > | as the first struct pid) is a disastrous idea. It destroys the
> >
> > True. Sorry, I did not mean we would need a new 'struct pid' for an
> > existing process. I think we talked earlier of finding a way of attaching
> > additional pid numbers to the same struct pid.
>
> I just played with this and if you make the semantics of unshare(CLONE_NEWPID)
> to be that you become the idle task aka pid 0, and not the init task pid 1 the
> implementation is trivial.
Heh, and then (browsing through your copy_process() patch hunks) the next
forked task becomes the child reaper for the new pidns? <shrug> why not
I guess.
Now if that child reaper then gets killed, will the idle task get killed too?
And if not, then idle task can just re-populating the new pidns with new
idle tasks...
If this brought us a step closer to entering an existing pidns that would
be one thing, but is there actually any advantage to being able to
unshare a new pidns? Oh, I guess there is - PAM can then use it at
login, which might be neat.
-serge
^ permalink raw reply
* Re: Current wireless-testing breaks libpcap: mr_alen should be set
From: Frank W. Miller @ 2010-03-03 15:31 UTC (permalink / raw)
To: tcpdump-workers, linux-wireless, netdev; +Cc: 'Jiri Pirko'
In-Reply-To: <1267578048.14049.11.camel@mj>
Would this be preventing pcap_inject() from working say in kernel 2.6.31
(stock FC12 kernel)?
Thanks,
FM
> -----Original Message-----
> From: tcpdump-workers-owner@lists.tcpdump.org [mailto:tcpdump-workers-
> owner@lists.tcpdump.org] On Behalf Of Pavel Roskin
> Sent: Tuesday, March 02, 2010 6:01 PM
> To: linux-wireless@vger.kernel.org; netdev@vger.kernel.org; tcpdump-
> workers@lists.tcpdump.org
> Cc: Jiri Pirko
> Subject: [tcpdump-workers] Current wireless-testing breaks libpcap:
> mr_alen should be set
>
> Hello!
>
> The current wireless-testing appears to have some non-wireless bits from
> the upcoming Linux 2.6.34. As a result, libpcap and all capture
> programs that use it are broken.
>
> This patch to libpcap helps:
>
> --- a/pcap-linux.c
> +++ b/pcap-linux.c
> @@ -1563,6 +1563,7 @@ live_open_new(pcap_t *handle, const char
> memset(&mr, 0, sizeof(mr));
> mr.mr_ifindex = handle->md.ifindex;
> mr.mr_type = PACKET_MR_PROMISC;
> + mr.mr_alen = 6;
> if (setsockopt(sock_fd, SOL_PACKET,
> PACKET_ADD_MEMBERSHIP, &mr, sizeof(mr)) ==
-1)
> {
>
> libpcap git doesn't have the fix yet.
>
> The breakage must be coming from the commit 914c8ad2 by Jiri Pirko to
> net/packet/af_packet.c
>
> I think it's very unhelpful to introduce patches that break significant
> userspace functionality without giving the affected programs an advance
> warning.
>
> Also, pulling bleeding edge stuff into wireless-testing before rc1
> appears to be either a mistake or a bad decision.
>
> Sorry for cross-post, but it's an urgent issue. Repliers are encouraged
> to trim the recipient list as necessary.
>
> --
> Regards,
> Pavel Roskin
> -
> This is the tcpdump-workers list.
> Visit https://cod.sandelman.ca/ to unsubscribe.
^ permalink raw reply
* Re: [patch] cassini: fix off by one
From: David Miller @ 2010-03-03 15:17 UTC (permalink / raw)
To: jaswinderlinux
Cc: error27, netdev, adobriyan, akpm, yanghy, kaber, linux-kernel,
kernel-janitors
In-Reply-To: <3f9a31f41003030710p4209fa1dq37d14a67b12657d@mail.gmail.com>
From: Jaswinder Singh Rajput <jaswinderlinux@gmail.com>
Date: Wed, 3 Mar 2010 20:40:18 +0530
> Hello Dave and Dan,
>
> On Wed, Mar 3, 2010 at 2:37 PM, David Miller <davem@davemloft.net> wrote:
>> From: Dan Carpenter <error27@gmail.com>
>> Date: Wed, 3 Mar 2010 11:22:41 +0300
>>
>>> There are only 6 link_modes.
>>>
>>> Signed-off-by: Dan Carpenter <error27@gmail.com>
>>
>> Applied.
>
> I am curious should we add these kind of email address in git log.
> error27 seems error to me ;-)
>
> Can we make Signed-off address little bit more sensible.
Some people have silly email addresses, that is their choice.
I can't see any reason to apply censorship to an email address
just because the character sequences in it have some kind of
meaning.
The email address I was using up until 6 or 7 years ago used a host
name that meant "shit" in the Polish languange.
Maybe I should have been censored too?
^ permalink raw reply
* Re: [patch] cassini: fix off by one
From: Jaswinder Singh Rajput @ 2010-03-03 15:10 UTC (permalink / raw)
To: David Miller
Cc: error27, netdev, adobriyan, akpm, yanghy, kaber, linux-kernel,
kernel-janitors
In-Reply-To: <20100303.010746.134683403.davem@davemloft.net>
Hello Dave and Dan,
On Wed, Mar 3, 2010 at 2:37 PM, David Miller <davem@davemloft.net> wrote:
> From: Dan Carpenter <error27@gmail.com>
> Date: Wed, 3 Mar 2010 11:22:41 +0300
>
>> There are only 6 link_modes.
>>
>> Signed-off-by: Dan Carpenter <error27@gmail.com>
>
> Applied.
I am curious should we add these kind of email address in git log.
error27 seems error to me ;-)
Can we make Signed-off address little bit more sensible.
Thanks,
--
Jaswinder
^ permalink raw reply
* Re: [rfc/rft][patch] should use scheduler sync hint in tcp_prequeue()?
From: Mike Galbraith @ 2010-03-03 14:46 UTC (permalink / raw)
To: David Miller; +Cc: eric.dumazet, netdev, linux-kernel, mingo
In-Reply-To: <20100303.060529.195713861.davem@davemloft.net>
On Wed, 2010-03-03 at 06:05 -0800, David Miller wrote:
> It definitely looks like an oversight to me and we should add
> be using wake_up_interruptible_sync_poll() in tcp_prequeue()
> as above.
>
> Ingo?
Implied ack from network maintainer implies ship it to me.
net: add scheduler sync hint to tcp_prequeue().
Decreases the odds wakee will suffer from frequent cache misses.
Signed-off-by: Mike Galbraith <efault@gmx.de>
Cc: David Miller <davem@davemloft.net>
LKML-Reference: <new-submission>
diff --git a/include/net/tcp.h b/include/net/tcp.h
index 34f5cc2..ba3fc64 100644
--- a/include/net/tcp.h
+++ b/include/net/tcp.h
@@ -939,7 +939,7 @@ static inline int tcp_prequeue(struct sock *sk, struct sk_buff *skb)
tp->ucopy.memory = 0;
} else if (skb_queue_len(&tp->ucopy.prequeue) == 1) {
- wake_up_interruptible_poll(sk->sk_sleep,
+ wake_up_interruptible_sync_poll(sk->sk_sleep,
POLLIN | POLLRDNORM | POLLRDBAND);
if (!inet_csk_ack_scheduled(sk))
inet_csk_reset_xmit_timer(sk, ICSK_TIME_DACK,
^ permalink raw reply related
* Re: [PATCH 2/2] ipv4: flush ARP entries on device change
From: Patrick McHardy @ 2010-03-03 14:44 UTC (permalink / raw)
To: Timo Teräs; +Cc: netdev
In-Reply-To: <4B8E74A8.5060600@iki.fi>
Timo Teräs wrote:
> Patrick McHardy wrote:
>> Timo Teräs wrote:
>>> Patrick McHardy wrote:
>>>> Timo Teras wrote:
>>>> It would be nice if we could restrict this to IFF_NOARP changes.
>>> Yes. But I did not see any easy way to figure out which flags have
>>> changed.
>>>
>>> Should we just keep a copy of the previous IFF_NOARP bit somewhere
>>> (where?).
>>> Or did I miss something obvious?
>>
>> We shouldn't have any arp entries for devices with IFF_NOARP set,
>> so perhaps we can flush only in that case. The transition IFF_NOARP
>> -> ~IFF_NOARP shouldn't need flushing.
>
> IFF_NOARP devices do have neighbor entries with the nud NOARP.
> Exactly those entries I want to flush when IFF_NOARP flag is
> removed.
>
> You can see those entries with "ip neigh show nud all". You have
> them e.g. for loopback stuff and broad-/multicast stuff in general.
> With IFF_NOARP you get them on all unicast addresses used.
I see. I don't have a better suggestion, except perhaps
to store the bit in dev->priv_flags.
^ permalink raw reply
* Re: [PATCH 2/2] ipv4: flush ARP entries on device change
From: Timo Teräs @ 2010-03-03 14:39 UTC (permalink / raw)
To: Patrick McHardy; +Cc: netdev
In-Reply-To: <4B8E7357.5050203@trash.net>
Patrick McHardy wrote:
> Timo Teräs wrote:
>> Patrick McHardy wrote:
>>> Timo Teras wrote:
>>> It would be nice if we could restrict this to IFF_NOARP changes.
>> Yes. But I did not see any easy way to figure out which flags have changed.
>>
>> Should we just keep a copy of the previous IFF_NOARP bit somewhere
>> (where?).
>> Or did I miss something obvious?
>
> We shouldn't have any arp entries for devices with IFF_NOARP set,
> so perhaps we can flush only in that case. The transition IFF_NOARP
> -> ~IFF_NOARP shouldn't need flushing.
IFF_NOARP devices do have neighbor entries with the nud NOARP.
Exactly those entries I want to flush when IFF_NOARP flag is
removed.
You can see those entries with "ip neigh show nud all". You have
them e.g. for loopback stuff and broad-/multicast stuff in general.
With IFF_NOARP you get them on all unicast addresses used.
- Timo
^ permalink raw reply
* Re: [PATCH V2 5/7] sctp: use limited socket backlog
From: Vlad Yasevich @ 2010-03-03 14:36 UTC (permalink / raw)
To: Zhu, Yi; +Cc: netdev@vger.kernel.org, Sridhar Samudrala
In-Reply-To: <DA586906BA1FFC4384FCFD6429ECE860A45A699B@shzsmsx502.ccr.corp.intel.com>
Zhu, Yi wrote:
> Vlad Yasevich <vladislav.yasevich@hp.com> wrote:
>
>> I think this will result in a double-free of the skb, because sctp_chunk_free
>> attempts to free the skb that's been assigned to the chunk. You can set the skb
>> to NULL to get around that.
>
> Ah, I missed that. Thanks!
>
> <...>
>
>> You also leak the ref counts here since now it's possible to not add a packet to
>> the backlog queue. That means you'll take refs, but never drop them because
>> the receive routing will never run.
>
> Good catch. I'll fix it.
>
> BTW, does the current backlog limit (sysctl_rmem_default[1] << 1) enough for sctp?
> I noticed the sysctl_sctp_rmem[1] is set to 373500 in my box.
>
sctp uses the same algorithm as TCP to figure out the memory values.
I guess the issue with using the smaller value that it would be possible to
queue more the socket receive buffer then to the backlog. Thus backlog would
start dropping packets even though receive buffer would still accept them.
-vlad
> Thanks,
> -yi
>
^ permalink raw reply
* Re: [PATCH 2/2] ipv4: flush ARP entries on device change
From: Patrick McHardy @ 2010-03-03 14:33 UTC (permalink / raw)
To: Timo Teräs; +Cc: netdev
In-Reply-To: <4B8E701D.7070300@iki.fi>
Timo Teräs wrote:
> Patrick McHardy wrote:
>> Timo Teras wrote:
>>> If device flag IFF_NOARP is changed, we should flush the ARP cache as
>>> all
>>> entries need to get refreshed.
>>>
>>> Signed-off-by: Timo Teras <timo.teras@iki.fi>
>>> ---
>>> net/ipv4/arp.c | 3 +++
>>> 1 files changed, 3 insertions(+), 0 deletions(-)
>>>
>>> diff --git a/net/ipv4/arp.c b/net/ipv4/arp.c
>>> index c4dd135..036da92 100644
>>> --- a/net/ipv4/arp.c
>>> +++ b/net/ipv4/arp.c
>>> @@ -1245,6 +1245,9 @@ static int arp_netdev_event(struct
>>> notifier_block *this, unsigned long event, vo
>>> neigh_changeaddr(&arp_tbl, dev);
>>> rt_cache_flush(dev_net(dev), 0);
>>> break;
>>> + case NETDEV_CHANGE:
>>> + neigh_changeaddr(&arp_tbl, dev);
>>> + break;
>>
>> It would be nice if we could restrict this to IFF_NOARP changes.
>
> Yes. But I did not see any easy way to figure out which flags have changed.
>
> Should we just keep a copy of the previous IFF_NOARP bit somewhere
> (where?).
> Or did I miss something obvious?
We shouldn't have any arp entries for devices with IFF_NOARP set,
so perhaps we can flush only in that case. The transition IFF_NOARP
-> ~IFF_NOARP shouldn't need flushing.
^ permalink raw reply
* RE: [PATCH V2 2/7] tcp: use limited socket backlog
From: Eric Dumazet @ 2010-03-03 14:31 UTC (permalink / raw)
To: Zhu, Yi
Cc: netdev@vger.kernel.org, David S. Miller, Alexey Kuznetsov,
Pekka Savola (ipv6), Patrick McHardy
In-Reply-To: <DA586906BA1FFC4384FCFD6429ECE860A45A6999@shzsmsx502.ccr.corp.intel.com>
Le mercredi 03 mars 2010 à 22:12 +0800, Zhu, Yi a écrit :
> Eric Dumazet [mailto:eric.dumazet@gmail.com] wrote:
> > Le mercredi 03 mars 2010 à 17:06 +0800, Zhu Yi a écrit :
>
> >> I simply follow how the code is originally written. As you can see,
> >> tcp_v4_do_rcv() doesn't always do so. And in the backlog queuing place,
> >> we don't even bother to check.
>
> > You add a new point where a packet can be dropped, this should be
> > accounted for, so that admins can have a clue whats going on.
>
> > Previously, packet was always queued, and dropped later (and accounted)
>
> In case of the skb doesn't have a MD5 option while we are expecting one, or we
> failed to find the sk for the skb connection request, etc, the skb is dropped silently in
> tcp_v4_do_rcv(). No?
Then its a separate bug. MD5 support added so many bugs its not even
funny.
Existing bugs are not an excuse for adding new ones, we try the reverse.
No ?
^ permalink raw reply
* RE: [PATCH 8/8] x25: use limited socket backlog
From: Eric Dumazet @ 2010-03-03 14:33 UTC (permalink / raw)
To: Zhu, Yi; +Cc: andrew hendry, netdev@vger.kernel.org
In-Reply-To: <DA586906BA1FFC4384FCFD6429ECE860A45A6992@shzsmsx502.ccr.corp.intel.com>
Le mercredi 03 mars 2010 à 22:00 +0800, Zhu, Yi a écrit :
> andrew hendry <andrew.hendry@gmail.com> wrote:
>
> > Will wait for the next spin and in the meantime think if there is way
> > to test it. x25 with no loopback and being so slow probably cant generate the same
> > as your UDP case.
>
> I didn't find a way to drop the packet correctly. So I didn't change any behavior in
> this patch. Nor did I do in the second spin. It will be fine if you also think x25 doesn't
> need to limit its backlog size.
So are we sure we cant flood X25 backlog, using X25 over IP ?
You discovered a _fatal_ flaw in backlog processing, we should close all
holes, not only UDP case. You can be sure many bad guys will inspect all
possibilities to bring down Linux hosts.
If you feel uncomfortable with a small limit, just stick a big one, like
256 packets, and you are 100% sure you wont break a protocol. If this
limit happens to be too small, we can change it later.
(No need to count bytes, since truesize includes kernel overhead, and
this overhead depends on 32/64 wide of host and kernel versions)
^ permalink raw reply
* [net-next-2.6 PATCH] e1000e: do not modify tx queue len on link speed change
From: franco @ 2010-03-03 14:31 UTC (permalink / raw)
To: Brandeburg, Jesse; +Cc: Netdev, Allan, Bruce W
e1000e would try to set appropriate values of tx_queue_len for
negotiated
link speeds overwriting the already set configuration of tx_queue_len in
the process. This removes all lines touching tx_queue_len from the
driver.
Signed-off-by: Franco Fichtner <franco@lastsummer.de>
---
drivers/net/e1000e/netdev.c | 11 +----------
1 files changed, 1 insertions(+), 10 deletions(-)
diff --git a/drivers/net/e1000e/netdev.c b/drivers/net/e1000e/netdev.c
index 88d54d3..e1cceb6 100644
--- a/drivers/net/e1000e/netdev.c
+++ b/drivers/net/e1000e/netdev.c
@@ -2289,8 +2289,6 @@ static void e1000_configure_tx(struct
e1000_adapter *adapter)
ew32(TCTL, tctl);
e1000e_config_collision_dist(hw);
-
- adapter->tx_queue_len = adapter->netdev->tx_queue_len;
}
/**
@@ -2877,7 +2875,6 @@ void e1000e_down(struct e1000_adapter *adapter)
del_timer_sync(&adapter->watchdog_timer);
del_timer_sync(&adapter->phy_info_timer);
- netdev->tx_queue_len = adapter->tx_queue_len;
netif_carrier_off(netdev);
adapter->link_speed = 0;
adapter->link_duplex = 0;
@@ -3588,21 +3585,15 @@ static void e1000_watchdog_task(struct
work_struct *work)
"link gets many collisions.\n");
}
- /*
- * tweak tx_queue_len according to speed/duplex
- * and adjust the timeout factor
- */
- netdev->tx_queue_len = adapter->tx_queue_len;
+ /* adjust timeout factor according to speed/duplex */
adapter->tx_timeout_factor = 1;
switch (adapter->link_speed) {
case SPEED_10:
txb2b = 0;
- netdev->tx_queue_len = 10;
adapter->tx_timeout_factor = 16;
break;
case SPEED_100:
txb2b = 0;
- netdev->tx_queue_len = 100;
adapter->tx_timeout_factor = 10;
break;
}
--
^ permalink raw reply related
* Re: e1000e: reset of tx_queue_len
From: Franco Fichtner @ 2010-03-03 14:22 UTC (permalink / raw)
To: Brandeburg, Jesse; +Cc: Franco Fichtner, Netdev, Allan, Bruce W
In-Reply-To: <alpine.WNT.2.00.1003021020160.2888@jbrandeb-desk1.amr.corp.intel.com>
Hi Jesse,
> Its possible we should just remove this code from the in-kernel
> driver, as I don't think many other drivers mess with the tx_queue_len
> either. We did introduce the code to fix some users complaints about the
> amount of work that can get queued up in the hardware with tx_queue_len =
> 1000 when at 10/100 speeds.
>
I've tested the changes you suggested and now tx_queue_len
is no longer reset. I have also prepared a patch for net-next
for you to sign or ack (or nack) because I'd like to see this go,
also.
Thanks again,
Franco
^ permalink raw reply
* Re: [PATCH 2/2] ipv4: flush ARP entries on device change
From: Timo Teräs @ 2010-03-03 14:20 UTC (permalink / raw)
To: Patrick McHardy; +Cc: netdev
In-Reply-To: <4B8E6CC8.3080701@trash.net>
Patrick McHardy wrote:
> Timo Teras wrote:
>> If device flag IFF_NOARP is changed, we should flush the ARP cache as all
>> entries need to get refreshed.
>>
>> Signed-off-by: Timo Teras <timo.teras@iki.fi>
>> ---
>> net/ipv4/arp.c | 3 +++
>> 1 files changed, 3 insertions(+), 0 deletions(-)
>>
>> diff --git a/net/ipv4/arp.c b/net/ipv4/arp.c
>> index c4dd135..036da92 100644
>> --- a/net/ipv4/arp.c
>> +++ b/net/ipv4/arp.c
>> @@ -1245,6 +1245,9 @@ static int arp_netdev_event(struct notifier_block *this, unsigned long event, vo
>> neigh_changeaddr(&arp_tbl, dev);
>> rt_cache_flush(dev_net(dev), 0);
>> break;
>> + case NETDEV_CHANGE:
>> + neigh_changeaddr(&arp_tbl, dev);
>> + break;
>
> It would be nice if we could restrict this to IFF_NOARP changes.
Yes. But I did not see any easy way to figure out which flags have changed.
Should we just keep a copy of the previous IFF_NOARP bit somewhere (where?).
Or did I miss something obvious?
- Timo
^ permalink raw reply
* RE: [PATCH V2 5/7] sctp: use limited socket backlog
From: Zhu, Yi @ 2010-03-03 14:19 UTC (permalink / raw)
To: Vlad Yasevich; +Cc: netdev@vger.kernel.org, Sridhar Samudrala
In-Reply-To: <4B8E6DD0.1020603@hp.com>
Vlad Yasevich <vladislav.yasevich@hp.com> wrote:
> I think this will result in a double-free of the skb, because sctp_chunk_free
> attempts to free the skb that's been assigned to the chunk. You can set the skb
> to NULL to get around that.
Ah, I missed that. Thanks!
<...>
> You also leak the ref counts here since now it's possible to not add a packet to
> the backlog queue. That means you'll take refs, but never drop them because
> the receive routing will never run.
Good catch. I'll fix it.
BTW, does the current backlog limit (sysctl_rmem_default[1] << 1) enough for sctp?
I noticed the sysctl_sctp_rmem[1] is set to 373500 in my box.
Thanks,
-yi
^ permalink raw reply
* RE: [PATCH V2 2/7] tcp: use limited socket backlog
From: Zhu, Yi @ 2010-03-03 14:12 UTC (permalink / raw)
To: Eric Dumazet
Cc: netdev@vger.kernel.org, David S. Miller, Alexey Kuznetsov,
Pekka Savola (ipv6), Patrick McHardy
In-Reply-To: <1267610843.2997.7.camel@edumazet-laptop>
Eric Dumazet [mailto:eric.dumazet@gmail.com] wrote:
> Le mercredi 03 mars 2010 à 17:06 +0800, Zhu Yi a écrit :
>> I simply follow how the code is originally written. As you can see,
>> tcp_v4_do_rcv() doesn't always do so. And in the backlog queuing place,
>> we don't even bother to check.
> You add a new point where a packet can be dropped, this should be
> accounted for, so that admins can have a clue whats going on.
> Previously, packet was always queued, and dropped later (and accounted)
In case of the skb doesn't have a MD5 option while we are expecting one, or we
failed to find the sk for the skb connection request, etc, the skb is dropped silently in
tcp_v4_do_rcv(). No?
> Not everybody runs drop monitor :)
Thanks,
-yi
^ permalink raw reply
* Re: [PATCH V2 5/7] sctp: use limited socket backlog
From: Vlad Yasevich @ 2010-03-03 14:10 UTC (permalink / raw)
To: Zhu Yi; +Cc: netdev, Sridhar Samudrala
In-Reply-To: <1267605389-7369-5-git-send-email-yi.zhu@intel.com>
Zhu Yi wrote:
> Make sctp adapt to the limited socket backlog change.
>
> Cc: Vlad Yasevich <vladislav.yasevich@hp.com>
> Cc: Sridhar Samudrala <sri@us.ibm.com>
> Signed-off-by: Zhu Yi <yi.zhu@intel.com>
> ---
> net/sctp/input.c | 12 ++++++++----
> 1 files changed, 8 insertions(+), 4 deletions(-)
>
> diff --git a/net/sctp/input.c b/net/sctp/input.c
> index c0c973e..20e69c3 100644
> --- a/net/sctp/input.c
> +++ b/net/sctp/input.c
> @@ -75,7 +75,7 @@ static struct sctp_association *__sctp_lookup_association(
> const union sctp_addr *peer,
> struct sctp_transport **pt);
>
> -static void sctp_add_backlog(struct sock *sk, struct sk_buff *skb);
> +static int sctp_add_backlog(struct sock *sk, struct sk_buff *skb);
>
>
> /* Calculate the SCTP checksum of an SCTP packet. */
> @@ -265,8 +265,12 @@ int sctp_rcv(struct sk_buff *skb)
> }
>
> if (sock_owned_by_user(sk)) {
> + if (sctp_add_backlog(sk, skb)) {
> + sctp_bh_unlock_sock(sk);
> + sctp_chunk_free(chunk);
> + goto discard_release;
> + }
I think this will result in a double-free of the skb, because sctp_chunk_free
attempts to free the skb that's been assigned to the chunk. You can set the skb
to NULL to get around that.
> SCTP_INC_STATS_BH(SCTP_MIB_IN_PKT_BACKLOG);
> - sctp_add_backlog(sk, skb);
> } else {
> SCTP_INC_STATS_BH(SCTP_MIB_IN_PKT_SOFTIRQ);
> sctp_inq_push(&chunk->rcvr->inqueue, chunk);
> @@ -362,7 +366,7 @@ done:
> return 0;
> }
>
> -static void sctp_add_backlog(struct sock *sk, struct sk_buff *skb)
> +static int sctp_add_backlog(struct sock *sk, struct sk_buff *skb)
> {
> struct sctp_chunk *chunk = SCTP_INPUT_CB(skb)->chunk;
> struct sctp_ep_common *rcvr = chunk->rcvr;
> @@ -377,7 +381,7 @@ static void sctp_add_backlog(struct sock *sk, struct sk_buff *skb)
> else
> BUG();
>
> - sk_add_backlog(sk, skb);
> + return sk_add_backlog_limited(sk, skb);
> }
You also leak the ref counts here since now it's possible to not add a packet to
the backlog queue. That means you'll take refs, but never drop them because
the receive routing will never run.
-vlad
>
> /* Handle icmp frag needed error. */
^ permalink raw reply
* Re: [PATCH net-next-2.6 v6 00/12] net-caif: introducing CAIF protocol stack
From: Marcel Holtmann @ 2010-03-03 14:10 UTC (permalink / raw)
To: David Miller
Cc: sjur.brandeland, netdev, daniel.martensson, kaber, stefano.babic,
randy.dunlap
In-Reply-To: <20100303.011711.218594734.davem@davemloft.net>
Hi Dave,
> I'll take this in during the next merge window, I'm taking
> bug fixes only at this time.
>
> But this does mean that there's now more time to make refinements
> and make it even more ready for inclusion.
too bad, I would have really liked to see this one going in.
Sjur, don't wait until the next merge window. Once Dave opens the
net-next tree for 2.6.35, lets get this in to finalize the constant
assignments.
Regards
Marcel
^ permalink raw reply
* Re: [PATCH 2/2] ipv4: flush ARP entries on device change
From: Patrick McHardy @ 2010-03-03 14:06 UTC (permalink / raw)
To: Timo Teras; +Cc: netdev
In-Reply-To: <1267624874-22326-2-git-send-email-timo.teras@iki.fi>
Timo Teras wrote:
> If device flag IFF_NOARP is changed, we should flush the ARP cache as all
> entries need to get refreshed.
>
> Signed-off-by: Timo Teras <timo.teras@iki.fi>
> ---
> net/ipv4/arp.c | 3 +++
> 1 files changed, 3 insertions(+), 0 deletions(-)
>
> diff --git a/net/ipv4/arp.c b/net/ipv4/arp.c
> index c4dd135..036da92 100644
> --- a/net/ipv4/arp.c
> +++ b/net/ipv4/arp.c
> @@ -1245,6 +1245,9 @@ static int arp_netdev_event(struct notifier_block *this, unsigned long event, vo
> neigh_changeaddr(&arp_tbl, dev);
> rt_cache_flush(dev_net(dev), 0);
> break;
> + case NETDEV_CHANGE:
> + neigh_changeaddr(&arp_tbl, dev);
> + break;
It would be nice if we could restrict this to IFF_NOARP changes.
^ permalink raw reply
* Re: [rfc/rft][patch] should use scheduler sync hint in tcp_prequeue()?
From: David Miller @ 2010-03-03 14:05 UTC (permalink / raw)
To: eric.dumazet; +Cc: efault, netdev, linux-kernel, mingo
In-Reply-To: <1267529273.2964.111.camel@edumazet-laptop>
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Tue, 02 Mar 2010 12:27:53 +0100
> Le mardi 02 mars 2010 à 10:41 +0100, Mike Galbraith a écrit :
>> diff --git a/include/net/tcp.h b/include/net/tcp.h
>> index 34f5cc2..ba3fc64 100644
>> --- a/include/net/tcp.h
>> +++ b/include/net/tcp.h
>> @@ -939,7 +939,7 @@ static inline int tcp_prequeue(struct sock *sk, struct sk_buff *skb)
>>
>> tp->ucopy.memory = 0;
>> } else if (skb_queue_len(&tp->ucopy.prequeue) == 1) {
>> - wake_up_interruptible_poll(sk->sk_sleep,
>> + wake_up_interruptible_sync_poll(sk->sk_sleep,
>> POLLIN | POLLRDNORM | POLLRDBAND);
>> if (!inet_csk_ack_scheduled(sk))
>> inet_csk_reset_xmit_timer(sk, ICSK_TIME_DACK,
>>
...
> Ingo probably can answer to this question, since he changed
> sock_def_readable() (and others) in commit 6f3d09291b498299
> I suspect he missed tcp_prequeue() case, maybe not...
It definitely looks like an oversight to me and we should add
be using wake_up_interruptible_sync_poll() in tcp_prequeue()
as above.
Ingo?
^ permalink raw reply
* [PATCH 2/2] ipv4: flush ARP entries on device change
From: Timo Teras @ 2010-03-03 14:01 UTC (permalink / raw)
To: netdev; +Cc: Timo Teras
In-Reply-To: <1267624874-22326-1-git-send-email-timo.teras@iki.fi>
If device flag IFF_NOARP is changed, we should flush the ARP cache as all
entries need to get refreshed.
Signed-off-by: Timo Teras <timo.teras@iki.fi>
---
net/ipv4/arp.c | 3 +++
1 files changed, 3 insertions(+), 0 deletions(-)
diff --git a/net/ipv4/arp.c b/net/ipv4/arp.c
index c4dd135..036da92 100644
--- a/net/ipv4/arp.c
+++ b/net/ipv4/arp.c
@@ -1245,6 +1245,9 @@ static int arp_netdev_event(struct notifier_block *this, unsigned long event, vo
neigh_changeaddr(&arp_tbl, dev);
rt_cache_flush(dev_net(dev), 0);
break;
+ case NETDEV_CHANGE:
+ neigh_changeaddr(&arp_tbl, dev);
+ break;
default:
break;
}
--
1.6.3.3
^ permalink raw reply related
* [PATCH 1/2] gre: fix hard header destination address checking
From: Timo Teras @ 2010-03-03 14:01 UTC (permalink / raw)
To: netdev; +Cc: Timo Teras
ipgre_header() can be called with zero daddr when the gre device is
configured as multipoint tunnel and still has the NOARP flag set (which is
typically cleared by the userspace arp daemon). If the NOARP packets are
not dropped, ipgre_tunnel_xmit() will take rt->rt_gateway (= NBMA IP) and
use that for route look up (and may lead to bogus xfrm acquires).
The multicast address check is removed as sending to multicast group should
be ok. In fact, if gre device has a multicast address as destination
ipgre_header is always called with multicast address.
Signed-off-by: Timo Teras <timo.teras@iki.fi>
---
net/ipv4/ip_gre.c | 7 ++-----
1 files changed, 2 insertions(+), 5 deletions(-)
diff --git a/net/ipv4/ip_gre.c b/net/ipv4/ip_gre.c
index c0c5274..f47c9f7 100644
--- a/net/ipv4/ip_gre.c
+++ b/net/ipv4/ip_gre.c
@@ -1144,12 +1144,9 @@ static int ipgre_header(struct sk_buff *skb, struct net_device *dev,
if (saddr)
memcpy(&iph->saddr, saddr, 4);
-
- if (daddr) {
+ if (daddr)
memcpy(&iph->daddr, daddr, 4);
- return t->hlen;
- }
- if (iph->daddr && !ipv4_is_multicast(iph->daddr))
+ if (iph->daddr)
return t->hlen;
return -t->hlen;
--
1.6.3.3
^ permalink raw reply related
* RE: [PATCH 8/8] x25: use limited socket backlog
From: Zhu, Yi @ 2010-03-03 14:00 UTC (permalink / raw)
To: andrew hendry, Eric Dumazet; +Cc: netdev@vger.kernel.org
In-Reply-To: <d45a3acc1003030338m3102a95p1b06b7db1b24e58e@mail.gmail.com>
andrew hendry <andrew.hendry@gmail.com> wrote:
> Will wait for the next spin and in the meantime think if there is way
> to test it. x25 with no loopback and being so slow probably cant generate the same
> as your UDP case.
I didn't find a way to drop the packet correctly. So I didn't change any behavior in
this patch. Nor did I do in the second spin. It will be fine if you also think x25 doesn't
need to limit its backlog size.
Thanks,
-yi
^ permalink raw reply
* [PATCH] rstp: Fix compiler type-punning error in rstp daemon
From: Neil Horman @ 2010-03-03 13:53 UTC (permalink / raw)
To: shemminger; +Cc: netdev
Hey-
Was trying to build the rstp daemon today and got a type-punning error
in bridge_track.c. This patch fixes it up.
Signed-off-by: Neil Horman <nhorman@tuxdriver.com>
bridge_track.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/bridge_track.c b/bridge_track.c
index be555de..fa59732 100644
--- a/bridge_track.c
+++ b/bridge_track.c
@@ -599,7 +599,7 @@ void bridge_bpdu_rcv(int if_index, const unsigned char *data, int len)
// TST(ntohs(*(uint16_t*)bpdu.body.message_age)
// < ntohs(*(uint16_t*)bpdu.body.max_age), );
TST(memcmp(bpdu->body.bridge_id, &ifc->master->bridge_id, 8) != 0
- || (ntohs(*(uint16_t *) bpdu->body.port_id) & 0xfff) !=
+ || (ntohs((uint16_t) *bpdu->body.port_id) & 0xfff) !=
ifc->port_index,);
break;
case BPDU_TOPO_CHANGE_TYPE:
^ permalink raw reply related
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