* Re: kernel stack trace using conntrack
From: Pablo Neira Ayuso @ 2010-02-16 13:33 UTC (permalink / raw)
To: Eric Dumazet; +Cc: Ramblewski David, netfilter-devel@vger.kernel.org, netdev
In-Reply-To: <1266318928.3045.38.camel@edumazet-laptop>
Eric Dumazet wrote:
> OK thanks David, I reproduced the problem on latest net-next-2.6 tree
> too. I wonder why nobody hit this before.
Hmm, my config had not NETFILTER_DEBUG enabled, that's why I didn't hit
that assertion.
> [352468.556484] ------------[ cut here ]------------
> [352468.556511] WARNING: at net/netfilter/nf_conntrack_extend.c:82
> __nf_ct_ext_add+0x1c2/0x1e0 [nf_conntrack]()
> [352468.556559] Hardware name: ProLiant BL460c G1
> [352468.556582] Modules linked in: nf_defrag_ipv4 nf_conntrack_netlink
> nf_conntrack sch_hfsc sch_sfq ipmi_devintf ipmi_si ipmi_msghandler hpilo
> bonding [last unloaded: nf_conntrack_ipv4]
> [352468.556675] Pid: 18852, comm: conntrack Tainted: G W
> 2.6.33-rc5-02754-g0ea034c-dirty #545
> [352468.556721] Call Trace:
> [352468.556742] [<c054d45f>] ? printk+0x1d/0x26
> [352468.556767] [<c023bbc2>] warn_slowpath_common+0x72/0xa0
> [352468.556795] [<fee75e42>] ? __nf_ct_ext_add+0x1c2/0x1e0
> [nf_conntrack]
> [352468.556825] [<fee75e42>] ? __nf_ct_ext_add+0x1c2/0x1e0
> [nf_conntrack]
> [352468.556854] [<c023bc0a>] warn_slowpath_null+0x1a/0x20
> [352468.556882] [<fee75e42>] __nf_ct_ext_add+0x1c2/0x1e0 [nf_conntrack]
> [352468.556911] [<fee70dcc>] ? nf_conntrack_alloc+0x10c/0x1a0
> [nf_conntrack]
> [352468.556940] [<feecaf59>] ctnetlink_create_conntrack+0x339/0x360
> [nf_conntrack_netlink]
> [352468.556987] [<feeca26b>] ? ctnetlink_parse_tuple+0x14b/0x1c0
> [nf_conntrack_netlink]
> [352468.557039] [<fee6fd60>] ? __nf_conntrack_find+0x70/0x100
> [nf_conntrack]
> [352468.557068] [<feecb090>] ctnetlink_new_conntrack+0x110/0x680
> [nf_conntrack_netlink]
> [352468.557113] [<c04d93b5>] nfnetlink_rcv_msg+0x125/0x180
> [352468.557140] [<c054ec57>] ? __mutex_lock_slowpath+0x197/0x230
> [352468.557167] [<c04d9290>] ? nfnetlink_rcv_msg+0x0/0x180
> [352468.557194] [<c04d5896>] netlink_rcv_skb+0x96/0xc0
> [352468.557219] [<c04d927c>] nfnetlink_rcv+0x1c/0x30
> [352468.557245] [<c04d5545>] netlink_unicast+0x255/0x2a0
> [352468.557274] [<c04d5d3f>] netlink_sendmsg+0x1af/0x2b0
> [352468.557300] [<c04a86ec>] sock_sendmsg+0xac/0xe0
> [352468.559358] [<c029d042>] ? find_get_page+0x22/0xd0
> [352468.559385] [<c029d9dc>] ? filemap_fault+0x8c/0x3c0
> [352468.559410] [<c04a905a>] sys_sendto+0xaa/0xd0
> [352468.559436] [<c02b3780>] ? __do_fault+0x370/0x470
> [352468.559462] [<c02b54d9>] ? handle_mm_fault+0x1d9/0x7d0
> [352468.559488] [<c04aa245>] sys_socketcall+0x195/0x280
> [352468.559514] [<c0202c50>] sysenter_do_call+0x12/0x26
> [352468.559539] ---[ end trace 6ecb842e4e35a653 ]---
>
> Could you try following patch ?
>
> Thanks
>
> diff --git a/net/netfilter/nf_conntrack_netlink.c b/net/netfilter/nf_conntrack_netlink.c
> index 0ffe689..d2657aa 100644
> --- a/net/netfilter/nf_conntrack_netlink.c
> +++ b/net/netfilter/nf_conntrack_netlink.c
> @@ -923,7 +923,7 @@ ctnetlink_change_status(struct nf_conn *ct, const struct nlattr * const cda[])
> unsigned int status = ntohl(nla_get_be32(cda[CTA_STATUS]));
> d = ct->status ^ status;
>
> - if (d & (IPS_EXPECTED|IPS_CONFIRMED|IPS_DYING))
> + if (d & (IPS_EXPECTED|IPS_DYING))
> /* unchangeable */
> return -EBUSY;
I think that we should explicitly report if the user unsets
IPS_CONFIRMED. Please, don't change this.
Apart from that, the patch seems fine to me. Thanks!
^ permalink raw reply
* Re: [net-next PATCH v4 3/3] net: reserve ports for applications using fixed port numbers
From: Eric Dumazet @ 2010-02-16 13:20 UTC (permalink / raw)
To: Cong Wang
Cc: Octavian Purdila, David Miller, Linux Kernel Network Developers,
Linux Kernel Developers, Neil Horman
In-Reply-To: <4B7A9852.5020105@redhat.com>
Le mardi 16 février 2010 à 21:06 +0800, Cong Wang a écrit :
> Octavian Purdila wrote:
> > On Tuesday 16 February 2010 11:37:04 you wrote:
> >>> BUILD_BUG_ON(sizeof(struct inet_skb_parm) > sizeof(dummy_skb->cb));
> >>>
> >>> + sysctl_local_reserved_ports = kzalloc(65536 / 8, GFP_KERNEL);
> >>> + if (!sysctl_local_reserved_ports)
> >>> + goto out;
> >>> +
> >> I think we should also consider the ports in ip_local_port_range,
> >> since we can only reserve the ports in that range.
> >>
> >
> > That is subject to changes at runtime, which means we will have to readjust
> > the bitmap at runtime which introduces the need for additional synchronization
> > operations which I would rather avoid.
>
> Why? As long as the bitmap is global, this will not be hard.
>
> Consider that if one user writes a port number which is beyond
> the ip_local_port_range into ip_local_reserved_ports, we should
> not accept this, because it doesn't make any sense. But with your
> patch, we do.
I disagree with you. This is perfectly OK.
A port not being flagged in ip_local_reserved_ports doesnt mean it can
be used for allocation.
If you want to really block ports from being used at boot, you could for
example :
# temporarly reduce the ip_local_port_range
echo "61000 61001" >/proc/sys/net/ipv4/ip_local_port_range
# Build our bitmap (could be slow, if a remote database is read)
for port in $LIST_RESERVED_PORT
do
echo $port >/proc/sys/net/ipv4/ip_local_reserved_ports
done
echo "10000 61000" >/proc/sys/net/ipv4/ip_local_port_range
^ permalink raw reply
* Re: [net-next-2.6 PATCH 1/7] xfrm: introduce basic mark infrastructure
From: jamal @ 2010-02-16 13:16 UTC (permalink / raw)
To: Benny Amorsen; +Cc: Patrick McHardy, timo.teras, herbert, davem, netdev
In-Reply-To: <m3hbphxrnm.fsf@ursa.amorsen.dk>
On Tue, 2010-02-16 at 13:59 +0100, Benny Amorsen wrote:
> From your description, I would add the IPSEC SPD + SA with a specific
> mark. Then I would set the mark in the rule table if I want the packets
> to go through the tunnel, or clear the mark to have them go through
> normal routing.
yes.
> Not perfect, because I would have to replicate parts of
> the routing table in the rule table, but it could be made to work.
Agreed this is a problem and not a nice one (the counter arguement is
at least theres a way for some users now..
> Perfect would be if I could set mark in the routing table instead of the
> rule table, but sometimes perfect is the enemy of good...
This is actually an interesting idea and is not far-fetched (and would
certainly get rid of the replication problem). If i understood
correctly, you would have:
ip route add blah blah mark 0x10
and that the routing core will use the mark to (as it does for example
with ifindex) to pick the route? I like the idea for the simple fact it
will reduce immensely configuration in some cases..
cheers,
jamal
^ permalink raw reply
* Re: xfrm: avoid spinlock in get_acqseq() used by xfrm user
From: Eric Dumazet @ 2010-02-16 13:09 UTC (permalink / raw)
To: hadi; +Cc: David Miller, netdev
In-Reply-To: <1266321682.6776.254.camel@bigi>
Le mardi 16 février 2010 à 07:01 -0500, jamal a écrit :
> Eric's version fixed it for pfkey. This one is for xfrm user.
> I thought about amortizing those two get_acqseq()s but it seems
> reasonable to have two of these sequence spaces for the two different
> interfaces.
>
Acked-by: Eric Dumazet <eric.dumazet@gmail.com>
Thanks !
^ permalink raw reply
* Re: [net-next PATCH v4 1/3] sysctl: refactor integer handling proc code
From: Cong Wang @ 2010-02-16 13:08 UTC (permalink / raw)
To: Octavian Purdila
Cc: David Miller, Linux Kernel Network Developers,
Linux Kernel Developers, Eric W. Biederman
In-Reply-To: <201002161248.56598.opurdila@ixiacom.com>
Octavian Purdila wrote:
> On Tuesday 16 February 2010 10:41:07 you wrote:
>
>>> +
>>> + if (!write && !first && left && !err)
>>> + err = proc_put_newline(&buffer, &left);
>>> + if (write && !err)
>>> + err = proc_skip_wspace(&buffer, &left);
>>> + if (err == -EFAULT /* do we really need to check for -EFAULT? */ ||
>>> + (write && first))
>>> + return err ? : -EINVAL;
>> The logic here seems messy, adding one or two goto's may help?
>>
>
> OK, I'll give it a try.
>
> What about the EFAULT check, is that really required?
I think so, it means to keep the errno to user-space when it is EFAULT,
right? This seems reasonable.
^ permalink raw reply
* Re: [net-next PATCH v4 1/3] sysctl: refactor integer handling proc code
From: Cong Wang @ 2010-02-16 13:09 UTC (permalink / raw)
To: Octavian Purdila
Cc: David Miller, Linux Kernel Network Developers,
Linux Kernel Developers, Eric W. Biederman
In-Reply-To: <201002161341.56206.opurdila@ixiacom.com>
Octavian Purdila wrote:
> On Tuesday 16 February 2010 10:41:07 you wrote:
>
>>> +static int proc_skip_wspace(char __user **buf, size_t *size)
>>> +{
>>> + char c;
>>> +
>>> + while (*size) {
>>> + if (get_user(c, *buf))
>>> + return -EFAULT;
>>> + if (!isspace(c))
>>> + break;
>>> + (*size)--; (*buf)++;
>>> + }
>>> +
>>> + return 0;
>>> +}
>> In lib/string.c we have skip_spaces(), I think we can use it
>> here instead of inventing another one.
>>
>
> I'm afraid we can't, skip_spaces does not accept userspace buffers.
Well, you need to use copy_from_user() before call it.
^ permalink raw reply
* Re: [net-next PATCH v4 3/3] net: reserve ports for applications using fixed port numbers
From: Cong Wang @ 2010-02-16 13:06 UTC (permalink / raw)
To: Octavian Purdila
Cc: David Miller, Linux Kernel Network Developers,
Linux Kernel Developers, Neil Horman, Eric Dumazet
In-Reply-To: <201002161306.29708.opurdila@ixiacom.com>
Octavian Purdila wrote:
> On Tuesday 16 February 2010 11:37:04 you wrote:
>>> BUILD_BUG_ON(sizeof(struct inet_skb_parm) > sizeof(dummy_skb->cb));
>>>
>>> + sysctl_local_reserved_ports = kzalloc(65536 / 8, GFP_KERNEL);
>>> + if (!sysctl_local_reserved_ports)
>>> + goto out;
>>> +
>> I think we should also consider the ports in ip_local_port_range,
>> since we can only reserve the ports in that range.
>>
>
> That is subject to changes at runtime, which means we will have to readjust
> the bitmap at runtime which introduces the need for additional synchronization
> operations which I would rather avoid.
Why? As long as the bitmap is global, this will not be hard.
Consider that if one user writes a port number which is beyond
the ip_local_port_range into ip_local_reserved_ports, we should
not accept this, because it doesn't make any sense. But with your
patch, we do.
>
>>> + {
>>> + .procname = "ip_local_reserved_ports",
>>> + .data = NULL, /* initialized in sysctl_ipv4_init */
>>> + .maxlen = 65536,
>>> + .mode = 0644,
>>> + .proc_handler = proc_dobitmap,
>>> + },
>> Isn't there an off-by-one here?
>>
>> In patch 2/3, you use 0 to set the fist bit, then how about 65535 which
>> writes 65536th bit? This is beyond the range of port number.
>>
>
> This seems fine to me, 65535 is the value used by both the port checking
> function and the proc read/write function. And it translates indeed to
> 65536th bit, but that is also bit 65535 if you start counting bits from 0
> instead of 1. The usual computing/natural arithmetic confusion for the meaning
> of first :)
>
Oh, I see.
Thanks.
^ permalink raw reply
* Re: [net-next-2.6 PATCH 1/7] xfrm: introduce basic mark infrastructure
From: Benny Amorsen @ 2010-02-16 12:59 UTC (permalink / raw)
To: hadi; +Cc: Patrick McHardy, timo.teras, herbert, davem, netdev
In-Reply-To: <1266321437.6776.250.camel@bigi>
jamal <hadi@cyberus.ca> writes:
> With these patches if you set policy routing mark, have the proper
> setting in the skb or socket for the mark then the proper
> route will be selected. If you have an SPD + SA added with the
> same mark, those will be used right after the route is selected.
> So essentially you have the same mark across.
> Does that solve or alleviate the problem?
I don't actually use marks at all, I do policy routing based on source
address. Currently rules are based on source interface, but all IPSEC
traffic comes from the same interface, unlike some tunnel-based
solutions.
Right now packets going out through an IPSEC tunnel do not hit the
routing table at all -- they just get shunted into the tunnel. Anything
that gives me the chance to run the packets through normal routing
before the tunnel grabs them works for me.
>From your description, I would add the IPSEC SPD + SA with a specific
mark. Then I would set the mark in the rule table if I want the packets
to go through the tunnel, or clear the mark to have them go through
normal routing. Not perfect, because I would have to replicate parts of
the routing table in the rule table, but it could be made to work.
Perfect would be if I could set mark in the routing table instead of the
rule table, but sometimes perfect is the enemy of good...
/Benny
^ permalink raw reply
* Re: [PATCH][v4] tcp: fix ICMP-RTO war
From: Ilpo Järvinen @ 2010-02-16 12:45 UTC (permalink / raw)
To: Damian Lukowski; +Cc: Netdev, David Miller
In-Reply-To: <4B7303C2.5000703@tvk.rwth-aachen.de>
First of all I want to let you know that I've no objection to this fix
itself (DaveM applied it already), it certainly does it's jobs in
preventing invalid < RTO_MIN state. However, I wonder if we could do
further improvements in this area...
On Wed, 10 Feb 2010, Damian Lukowski wrote:
> Make sure, that TCP has a nonzero RTT estimation after three-way
> handshake. Currently, a listening TCP has a value of 0 for srtt,
> rttvar and rto right after the three-way handshake is completed
> with TCP timestamps disabled.
> This will lead to corrupt RTO recalculation and retransmission
> flood when RTO is recalculated on backoff reversion as introduced
> in "Revert RTO on ICMP destination unreachable"
> (f1ecd5d9e7366609d640ff4040304ea197fbc618).
> This behaviour can be provoked by connecting to a server which
> "responds first" (like SMTP) and rejecting every packet after
> the handshake with dest-unreachable, which will lead to softirq
> load on the server (up to 30% per socket in some tests).
>
> Thanks to Ilpo Jarvinen for providing debug patches and to
> Denys Fedoryshchenko for reporting and testing.
>
> Changes since v3: Removed bad characters in patchfile.
>
> Reported-by: Denys Fedoryshchenko <denys@visp.net.lb>
> Signed-off-by: Damian Lukowski <damian@tvk.rwth-aachen.de>
> ---
> net/ipv4/tcp_input.c | 8 +++-----
> 1 files changed, 3 insertions(+), 5 deletions(-)
>
> diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
> index 28e0296..da7173b 100644
> --- a/net/ipv4/tcp_input.c
> +++ b/net/ipv4/tcp_input.c
> @@ -5783,12 +5783,10 @@ int tcp_rcv_state_process(struct sock *sk, struct sk_buff *skb,
>
> /* tcp_ack considers this ACK as duplicate
> * and does not calculate rtt.
> - * Fix it at least with timestamps.
> + * Force it here.
> */
> - if (tp->rx_opt.saw_tstamp &&
> - tp->rx_opt.rcv_tsecr && !tp->srtt)
> - tcp_ack_saw_tstamp(sk, 0);
> -
> + tcp_ack_update_rtt(sk, 0, 0);
> +
...Here a zero seq_rtt is given to RTT estimator (it will be effective
only in the case w/o timestamps, TS case recalculates it from the stored
timestamps). Maybe we could use some field (timestamp related one comes to
my mind) in request sock to get a real RTT estimate for non-timestamp case
too. ...It seems possible to me, though tricky because the request_sock is
no longer that easily available here so some parameter passing would be
needed.
--
i.
^ permalink raw reply
* xfrm: avoid spinlock in get_acqseq() used by xfrm user
From: jamal @ 2010-02-16 12:01 UTC (permalink / raw)
To: David Miller; +Cc: Eric Dumazet, netdev
[-- Attachment #1: Type: text/plain, Size: 224 bytes --]
Eric's version fixed it for pfkey. This one is for xfrm user.
I thought about amortizing those two get_acqseq()s but it seems
reasonable to have two of these sequence spaces for the two different
interfaces.
cheers,
jamal
[-- Attachment #2: xfrmacqseq --]
[-- Type: text/plain, Size: 972 bytes --]
commit d5168d5addbc999c94aacda8f28a4a173756a72b
Author: Jamal Hadi Salim <hadi@cyberus.ca>
Date: Tue Feb 16 06:51:22 2010 -0500
xfrm: avoid spinlock in get_acqseq() used by xfrm user
This is in the same spirit as commit 28aecb9d7728dc26bf03ce7925fe622023a83a2a
by Eric Dumazet.
Use atomic_inc_return() in get_acqseq() to avoid taking a spinlock
Signed-off-by: Jamal Hadi Salim <hadi@cyberus.ca>
diff --git a/net/xfrm/xfrm_state.c b/net/xfrm/xfrm_state.c
index f50ee9b..96f2088 100644
--- a/net/xfrm/xfrm_state.c
+++ b/net/xfrm/xfrm_state.c
@@ -1462,12 +1462,12 @@ EXPORT_SYMBOL(xfrm_find_acq_byseq);
u32 xfrm_get_acqseq(void)
{
u32 res;
- static u32 acqseq;
- static DEFINE_SPINLOCK(acqseq_lock);
+ static atomic_t acqseq;
+
+ do {
+ res = atomic_inc_return(&acqseq);
+ } while (!res);
- spin_lock_bh(&acqseq_lock);
- res = (++acqseq ? : ++acqseq);
- spin_unlock_bh(&acqseq_lock);
return res;
}
EXPORT_SYMBOL(xfrm_get_acqseq);
^ permalink raw reply related
* Re: [net-next-2.6 PATCH 1/7] xfrm: introduce basic mark infrastructure
From: jamal @ 2010-02-16 11:57 UTC (permalink / raw)
To: Benny Amorsen; +Cc: Patrick McHardy, timo.teras, herbert, davem, netdev
In-Reply-To: <m37hqdcvgm.fsf@ursa.amorsen.dk>
On Tue, 2010-02-16 at 11:43 +0100, Benny Amorsen wrote:
> xfrm ignores policy routing. You can't route IPSEC in Linux. This is
> actually a fairly annoying limitation. The workaround is to do like
> Microsoft: Encapsulate everything in l2tp or gre.
With these patches if you set policy routing mark, have the proper
setting in the skb or socket for the mark then the proper
route will be selected. If you have an SPD + SA added with the
same mark, those will be used right after the route is selected.
So essentially you have the same mark across.
Does that solve or alleviate the problem?
cheers,
jamal
^ permalink raw reply
* Re: [net-next PATCH v4 1/3] sysctl: refactor integer handling proc code
From: Octavian Purdila @ 2010-02-16 11:41 UTC (permalink / raw)
To: Cong Wang
Cc: David Miller, Linux Kernel Network Developers,
Linux Kernel Developers, Eric W. Biederman
In-Reply-To: <4B7A5A23.3080309@redhat.com>
On Tuesday 16 February 2010 10:41:07 you wrote:
> > +static int proc_skip_wspace(char __user **buf, size_t *size)
> > +{
> > + char c;
> > +
> > + while (*size) {
> > + if (get_user(c, *buf))
> > + return -EFAULT;
> > + if (!isspace(c))
> > + break;
> > + (*size)--; (*buf)++;
> > + }
> > +
> > + return 0;
> > +}
>
> In lib/string.c we have skip_spaces(), I think we can use it
> here instead of inventing another one.
>
I'm afraid we can't, skip_spaces does not accept userspace buffers.
^ permalink raw reply
* Re: [PATCH net-2.6] net: bug fix for vlan + gro issue
From: Herbert Xu @ 2010-02-16 11:40 UTC (permalink / raw)
To: Ajit Khaparde; +Cc: davem, kaber, netdev
In-Reply-To: <20100212053451.GA23351@serverengines.com>
On Fri, Feb 12, 2010 at 11:04:53AM +0530, Ajit Khaparde wrote:
>
> diff --git a/net/core/dev.c b/net/core/dev.c
> index be9924f..16aac06 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -2761,7 +2761,10 @@ gro_result_t napi_frags_finish(struct napi_struct *napi, struct sk_buff *skb,
> switch (ret) {
> case GRO_NORMAL:
> case GRO_HELD:
> - skb->protocol = eth_type_trans(skb, napi->dev);
> + if (skb->vlan_tci)
> + skb->protocol = eth_type_trans(skb, skb->dev);
> + else
> + skb->protocol = eth_type_trans(skb, napi->dev);
We should be able to just use skb->dev here.
Thanks!
--
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply
* RE: kernel stack trace using conntrack
From: Eric Dumazet @ 2010-02-16 11:15 UTC (permalink / raw)
To: Ramblewski David; +Cc: netfilter-devel@vger.kernel.org, netdev
In-Reply-To: <7EF5DBE4C76A7B4DA655334E9F2BFD26CED7BC8D54@FRSPX100.fr01.awl.atosorigin.net>
Le mardi 16 février 2010 à 11:25 +0100, Ramblewski David a écrit :
> Here it is:
>
> [root@obench03s ~]# uname -a
> Linux obench03s 2.6.32.8 #1 SMP PREEMPT Wed Feb 10 17:32:16 CET 2010 i686 i686 i386 GNU/Linux
>
> [root@obench03s ~]# lsmod
> Module Size Used by
> nf_conntrack_netlink 9810 0
> nf_conntrack 50283 1 nf_conntrack_netlink
> bonding 69153 0
> usbhid 14557 0
> ipmi_si 29902 0
> rtc_cmos 7175 0
> bnx2 53497 0
> hpwdt 5548 0
> rtc_core 11461 1 rtc_cmos
> rtc_lib 2022 1 rtc_core
> hpilo 6051 0
> ipmi_msghandler 27670 1 ipmi_si
> ehci_hcd 27584 0
> uhci_hcd 16357 0
> usbcore 120714 4 usbhid,ehci_hcd,uhci_hcd
> dm_mod 49320 0
> [root@obench03s ~]#
>
>
> [root@obench03s ~]# iptables --version
> iptables v1.4.0
> [root@obench03s ~]# iptables -L
> Chain INPUT (policy ACCEPT)
> target prot opt source destination
>
> Chain FORWARD (policy ACCEPT)
> target prot opt source destination
>
> Chain OUTPUT (policy ACCEPT)
> target prot opt source destination
> [root@obench03s ~]# iptables -L -t nat
> Chain PREROUTING (policy ACCEPT)
> target prot opt source destination
>
> Chain POSTROUTING (policy ACCEPT)
> target prot opt source destination
>
> Chain OUTPUT (policy ACCEPT)
> target prot opt source destination
> [root@obench03s ~]# conntrack -L
> udp 17 27 src=10.24.230.151 dst=10.26.103.28 sport=510 dport=516 packets=5 bytes=295 [UNREPLIED] src=10.26.103.28 dst=10.24.230.151 sport=516 dport=510 packets=0 bytes=0 mark=0 secmark=0 use=2
> tcp 6 299 ESTABLISHED src=10.26.103.28 dst=10.28.64.65 sport=22 dport=53497 packets=89 bytes=8628 src=10.28.64.65 dst=10.26.103.28 sport=53497 dport=22 packets=127 bytes=10396 [ASSURED] mark=0 secmark=0 use=2
> udp 17 21 src=127.0.0.1 dst=127.0.0.1 sport=32070 dport=161 packets=6 bytes=492 [UNREPLIED] src=127.0.0.1 dst=127.0.0.1 sport=161 dport=32070 packets=0 bytes=0 mark=0 secmark=0 use=2
> conntrack v0.9.14 (conntrack-tools): 3 flow entries have been shown.
> [root@obench03s ~]#
>
> David
OK thanks David, I reproduced the problem on latest net-next-2.6 tree
too. I wonder why nobody hit this before.
[352468.556484] ------------[ cut here ]------------
[352468.556511] WARNING: at net/netfilter/nf_conntrack_extend.c:82
__nf_ct_ext_add+0x1c2/0x1e0 [nf_conntrack]()
[352468.556559] Hardware name: ProLiant BL460c G1
[352468.556582] Modules linked in: nf_defrag_ipv4 nf_conntrack_netlink
nf_conntrack sch_hfsc sch_sfq ipmi_devintf ipmi_si ipmi_msghandler hpilo
bonding [last unloaded: nf_conntrack_ipv4]
[352468.556675] Pid: 18852, comm: conntrack Tainted: G W
2.6.33-rc5-02754-g0ea034c-dirty #545
[352468.556721] Call Trace:
[352468.556742] [<c054d45f>] ? printk+0x1d/0x26
[352468.556767] [<c023bbc2>] warn_slowpath_common+0x72/0xa0
[352468.556795] [<fee75e42>] ? __nf_ct_ext_add+0x1c2/0x1e0
[nf_conntrack]
[352468.556825] [<fee75e42>] ? __nf_ct_ext_add+0x1c2/0x1e0
[nf_conntrack]
[352468.556854] [<c023bc0a>] warn_slowpath_null+0x1a/0x20
[352468.556882] [<fee75e42>] __nf_ct_ext_add+0x1c2/0x1e0 [nf_conntrack]
[352468.556911] [<fee70dcc>] ? nf_conntrack_alloc+0x10c/0x1a0
[nf_conntrack]
[352468.556940] [<feecaf59>] ctnetlink_create_conntrack+0x339/0x360
[nf_conntrack_netlink]
[352468.556987] [<feeca26b>] ? ctnetlink_parse_tuple+0x14b/0x1c0
[nf_conntrack_netlink]
[352468.557039] [<fee6fd60>] ? __nf_conntrack_find+0x70/0x100
[nf_conntrack]
[352468.557068] [<feecb090>] ctnetlink_new_conntrack+0x110/0x680
[nf_conntrack_netlink]
[352468.557113] [<c04d93b5>] nfnetlink_rcv_msg+0x125/0x180
[352468.557140] [<c054ec57>] ? __mutex_lock_slowpath+0x197/0x230
[352468.557167] [<c04d9290>] ? nfnetlink_rcv_msg+0x0/0x180
[352468.557194] [<c04d5896>] netlink_rcv_skb+0x96/0xc0
[352468.557219] [<c04d927c>] nfnetlink_rcv+0x1c/0x30
[352468.557245] [<c04d5545>] netlink_unicast+0x255/0x2a0
[352468.557274] [<c04d5d3f>] netlink_sendmsg+0x1af/0x2b0
[352468.557300] [<c04a86ec>] sock_sendmsg+0xac/0xe0
[352468.559358] [<c029d042>] ? find_get_page+0x22/0xd0
[352468.559385] [<c029d9dc>] ? filemap_fault+0x8c/0x3c0
[352468.559410] [<c04a905a>] sys_sendto+0xaa/0xd0
[352468.559436] [<c02b3780>] ? __do_fault+0x370/0x470
[352468.559462] [<c02b54d9>] ? handle_mm_fault+0x1d9/0x7d0
[352468.559488] [<c04aa245>] sys_socketcall+0x195/0x280
[352468.559514] [<c0202c50>] sysenter_do_call+0x12/0x26
[352468.559539] ---[ end trace 6ecb842e4e35a653 ]---
Could you try following patch ?
Thanks
diff --git a/net/netfilter/nf_conntrack_netlink.c b/net/netfilter/nf_conntrack_netlink.c
index 0ffe689..d2657aa 100644
--- a/net/netfilter/nf_conntrack_netlink.c
+++ b/net/netfilter/nf_conntrack_netlink.c
@@ -923,7 +923,7 @@ ctnetlink_change_status(struct nf_conn *ct, const struct nlattr * const cda[])
unsigned int status = ntohl(nla_get_be32(cda[CTA_STATUS]));
d = ct->status ^ status;
- if (d & (IPS_EXPECTED|IPS_CONFIRMED|IPS_DYING))
+ if (d & (IPS_EXPECTED|IPS_DYING))
/* unchangeable */
return -EBUSY;
@@ -938,7 +938,7 @@ ctnetlink_change_status(struct nf_conn *ct, const struct nlattr * const cda[])
/* Be careful here, modifying NAT bits can screw up things,
* so don't let users modify them directly if they don't pass
* nf_nat_range. */
- ct->status |= status & ~(IPS_NAT_DONE_MASK | IPS_NAT_MASK);
+ ct->status |= status & ~(IPS_NAT_DONE_MASK | IPS_NAT_MASK | IPS_CONFIRMED);
return 0;
}
@@ -1193,7 +1193,6 @@ ctnetlink_create_conntrack(const struct nlattr * const cda[],
ct->timeout.expires = ntohl(nla_get_be32(cda[CTA_TIMEOUT]));
ct->timeout.expires = jiffies + ct->timeout.expires * HZ;
- ct->status |= IPS_CONFIRMED;
rcu_read_lock();
if (cda[CTA_HELP]) {
@@ -1296,6 +1295,7 @@ ctnetlink_create_conntrack(const struct nlattr * const cda[],
}
add_timer(&ct->timeout);
+ ct->status |= IPS_CONFIRMED;
nf_conntrack_hash_insert(ct);
rcu_read_unlock();
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* Re: [net-next PATCH v4 3/3] net: reserve ports for applications using fixed port numbers
From: Octavian Purdila @ 2010-02-16 11:06 UTC (permalink / raw)
To: Cong Wang
Cc: David Miller, Linux Kernel Network Developers,
Linux Kernel Developers, Neil Horman, Eric Dumazet
In-Reply-To: <4B7A6740.1000701@redhat.com>
On Tuesday 16 February 2010 11:37:04 you wrote:
> > BUILD_BUG_ON(sizeof(struct inet_skb_parm) > sizeof(dummy_skb->cb));
> >
> > + sysctl_local_reserved_ports = kzalloc(65536 / 8, GFP_KERNEL);
> > + if (!sysctl_local_reserved_ports)
> > + goto out;
> > +
>
> I think we should also consider the ports in ip_local_port_range,
> since we can only reserve the ports in that range.
>
That is subject to changes at runtime, which means we will have to readjust
the bitmap at runtime which introduces the need for additional synchronization
operations which I would rather avoid.
> > + {
> > + .procname = "ip_local_reserved_ports",
> > + .data = NULL, /* initialized in sysctl_ipv4_init */
> > + .maxlen = 65536,
> > + .mode = 0644,
> > + .proc_handler = proc_dobitmap,
> > + },
>
> Isn't there an off-by-one here?
>
> In patch 2/3, you use 0 to set the fist bit, then how about 65535 which
> writes 65536th bit? This is beyond the range of port number.
>
This seems fine to me, 65535 is the value used by both the port checking
function and the proc read/write function. And it translates indeed to
65536th bit, but that is also bit 65535 if you start counting bits from 0
instead of 1. The usual computing/natural arithmetic confusion for the meaning
of first :)
^ permalink raw reply
* Re: [net-next PATCH v4 1/3] sysctl: refactor integer handling proc code
From: Octavian Purdila @ 2010-02-16 10:48 UTC (permalink / raw)
To: Cong Wang
Cc: David Miller, Linux Kernel Network Developers,
Linux Kernel Developers, Eric W. Biederman
In-Reply-To: <4B7A5A23.3080309@redhat.com>
On Tuesday 16 February 2010 10:41:07 you wrote:
> > +
> > + if (!write && !first && left && !err)
> > + err = proc_put_newline(&buffer, &left);
> > + if (write && !err)
> > + err = proc_skip_wspace(&buffer, &left);
> > + if (err == -EFAULT /* do we really need to check for -EFAULT? */ ||
> > + (write && first))
> > + return err ? : -EINVAL;
>
> The logic here seems messy, adding one or two goto's may help?
>
OK, I'll give it a try.
What about the EFAULT check, is that really required?
^ permalink raw reply
* Re: [net-next-2.6 PATCH 1/7] xfrm: introduce basic mark infrastructure
From: Benny Amorsen @ 2010-02-16 10:43 UTC (permalink / raw)
To: hadi; +Cc: Patrick McHardy, timo.teras, herbert, davem, netdev
In-Reply-To: <1266254073.6776.109.camel@bigi>
jamal <hadi@cyberus.ca> writes:
> I take it policy routing by mark is insufficient.
xfrm ignores policy routing. You can't route IPSEC in Linux. This is
actually a fairly annoying limitation. The workaround is to do like
Microsoft: Encapsulate everything in l2tp or gre.
/Benny
^ permalink raw reply
* [PATCH net-2.6] be2net: set proper value to version field in req hdr
From: Ajit Khaparde @ 2010-02-16 10:18 UTC (permalink / raw)
To: davem, netdev
Before sending a command to the ASIC, set version properly.
This is necessary for the ARM firmware to send correct data to the driver.
This also fixes a bug in certain skews of the ASIC where the statistics
are misreported.
Signed-off-by: Ajit Khaparde <ajitk@serverengines.com>
---
drivers/net/benet/be_cmds.c | 1 +
1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/drivers/net/benet/be_cmds.c b/drivers/net/benet/be_cmds.c
index fee6eee..006cb2e 100644
--- a/drivers/net/benet/be_cmds.c
+++ b/drivers/net/benet/be_cmds.c
@@ -296,6 +296,7 @@ static void be_cmd_hdr_prepare(struct be_cmd_req_hdr *req_hdr,
req_hdr->opcode = opcode;
req_hdr->subsystem = subsystem;
req_hdr->request_length = cpu_to_le32(cmd_len - sizeof(*req_hdr));
+ req_hdr->version = 0;
}
static void be_cmd_page_addrs_prepare(struct phys_addr *pages, u32 max_pages,
--
1.6.3.3
^ permalink raw reply related
* Re: [net-next PATCH v4 3/3] net: reserve ports for applications using fixed port numbers
From: Cong Wang @ 2010-02-16 9:37 UTC (permalink / raw)
To: Octavian Purdila
Cc: David Miller, Linux Kernel Network Developers,
Linux Kernel Developers, Neil Horman, Eric Dumazet
In-Reply-To: <1266271241-6293-4-git-send-email-opurdila@ixiacom.com>
Octavian Purdila wrote:
> This patch introduces /proc/sys/net/ipv4/ip_local_reserved_ports
> (bitmap type) which allows users to reserve ports for third-party
> applications.
>
> The reserved ports will not be used by automatic port assignments
> (e.g. when calling connect() or bind() with port number 0). Explicit
> port allocation behavior is unchanged.
>
> Signed-off-by: Octavian Purdila <opurdila@ixiacom.com>
> Signed-off-by: WANG Cong <amwang@redhat.com>
> Cc: Neil Horman <nhorman@tuxdriver.com>
> Cc: Eric Dumazet <eric.dumazet@gmail.com>
> ---
> Documentation/networking/ip-sysctl.txt | 12 ++++++++++++
> drivers/infiniband/core/cma.c | 7 ++++++-
> include/net/ip.h | 6 ++++++
> net/ipv4/af_inet.c | 8 +++++++-
> net/ipv4/inet_connection_sock.c | 6 ++++++
> net/ipv4/inet_hashtables.c | 2 ++
> net/ipv4/sysctl_net_ipv4.c | 17 +++++++++++++++++
> net/ipv4/udp.c | 3 ++-
> net/sctp/socket.c | 2 ++
> 9 files changed, 60 insertions(+), 3 deletions(-)
>
> diff --git a/Documentation/networking/ip-sysctl.txt b/Documentation/networking/ip-sysctl.txt
> index 2dc7a1d..23be7a4 100644
> --- a/Documentation/networking/ip-sysctl.txt
> +++ b/Documentation/networking/ip-sysctl.txt
> @@ -564,6 +564,18 @@ ip_local_port_range - 2 INTEGERS
> (i.e. by default) range 1024-4999 is enough to issue up to
> 2000 connections per second to systems supporting timestamps.
>
> +ip_local_reserved_ports - BITMAP of 65536 ports
> + Specify the ports which are reserved for known third-party
> + applications. These ports will not be used by automatic port assignments
> + (e.g. when calling connect() or bind() with port number 0). Explicit
> + port allocation behavior is unchanged.
> +
> + Reserving ports is done by writing positive numbers in this proc entry,
> + clearing them is done by writing negative numbers (e.g. 8080 reserves
> + port number, -8080 makes it available for automatic assignment again).
> +
> + Default: Empty
> +
> ip_nonlocal_bind - BOOLEAN
> If set, allows processes to bind() to non-local IP addresses,
> which can be quite useful - but may break some applications.
> diff --git a/drivers/infiniband/core/cma.c b/drivers/infiniband/core/cma.c
> index cc9b594..8248fc6 100644
> --- a/drivers/infiniband/core/cma.c
> +++ b/drivers/infiniband/core/cma.c
> @@ -1979,6 +1979,8 @@ retry:
> /* FIXME: add proper port randomization per like inet_csk_get_port */
> do {
> ret = idr_get_new_above(ps, bind_list, next_port, &port);
> + if (inet_is_reserved_local_port(port))
> + ret = -EAGAIN;
> } while ((ret == -EAGAIN) && idr_pre_get(ps, GFP_KERNEL));
>
> if (ret)
> @@ -2997,10 +2999,13 @@ static int __init cma_init(void)
> {
> int ret, low, high, remaining;
>
> - get_random_bytes(&next_port, sizeof next_port);
> inet_get_local_port_range(&low, &high);
> +again:
> + get_random_bytes(&next_port, sizeof next_port);
> remaining = (high - low) + 1;
> next_port = ((unsigned int) next_port % remaining) + low;
> + if (inet_is_reserved_local_port(next_port))
> + goto again;
>
> cma_wq = create_singlethread_workqueue("rdma_cm");
> if (!cma_wq)
> diff --git a/include/net/ip.h b/include/net/ip.h
> index fb63371..2e24256 100644
> --- a/include/net/ip.h
> +++ b/include/net/ip.h
> @@ -184,6 +184,12 @@ extern struct local_ports {
> } sysctl_local_ports;
> extern void inet_get_local_port_range(int *low, int *high);
>
> +extern unsigned long *sysctl_local_reserved_ports;
> +static inline int inet_is_reserved_local_port(int port)
> +{
> + return test_bit(port, sysctl_local_reserved_ports);
> +}
> +
> extern int sysctl_ip_default_ttl;
> extern int sysctl_ip_nonlocal_bind;
>
> diff --git a/net/ipv4/af_inet.c b/net/ipv4/af_inet.c
> index 7d12c6a..06810b0 100644
> --- a/net/ipv4/af_inet.c
> +++ b/net/ipv4/af_inet.c
> @@ -1546,9 +1546,13 @@ static int __init inet_init(void)
>
> BUILD_BUG_ON(sizeof(struct inet_skb_parm) > sizeof(dummy_skb->cb));
>
> + sysctl_local_reserved_ports = kzalloc(65536 / 8, GFP_KERNEL);
> + if (!sysctl_local_reserved_ports)
> + goto out;
> +
I think we should also consider the ports in ip_local_port_range,
since we can only reserve the ports in that range.
> rc = proto_register(&tcp_prot, 1);
> if (rc)
> - goto out;
> + goto out_free_reserved_ports;
>
> rc = proto_register(&udp_prot, 1);
> if (rc)
> @@ -1647,6 +1651,8 @@ out_unregister_udp_proto:
> proto_unregister(&udp_prot);
> out_unregister_tcp_proto:
> proto_unregister(&tcp_prot);
> +out_free_reserved_ports:
> + kfree(sysctl_local_reserved_ports);
> goto out;
> }
>
> diff --git a/net/ipv4/inet_connection_sock.c b/net/ipv4/inet_connection_sock.c
> index 8da6429..1acb462 100644
> --- a/net/ipv4/inet_connection_sock.c
> +++ b/net/ipv4/inet_connection_sock.c
> @@ -37,6 +37,9 @@ struct local_ports sysctl_local_ports __read_mostly = {
> .range = { 32768, 61000 },
> };
>
> +unsigned long *sysctl_local_reserved_ports;
> +EXPORT_SYMBOL(sysctl_local_reserved_ports);
> +
> void inet_get_local_port_range(int *low, int *high)
> {
> unsigned seq;
> @@ -108,6 +111,8 @@ again:
>
> smallest_size = -1;
> do {
> + if (inet_is_reserved_local_port(rover))
> + goto next_nolock;
> head = &hashinfo->bhash[inet_bhashfn(net, rover,
> hashinfo->bhash_size)];
> spin_lock(&head->lock);
> @@ -130,6 +135,7 @@ again:
> break;
> next:
> spin_unlock(&head->lock);
> + next_nolock:
> if (++rover > high)
> rover = low;
> } while (--remaining > 0);
> diff --git a/net/ipv4/inet_hashtables.c b/net/ipv4/inet_hashtables.c
> index 2b79377..d3e160a 100644
> --- a/net/ipv4/inet_hashtables.c
> +++ b/net/ipv4/inet_hashtables.c
> @@ -456,6 +456,8 @@ int __inet_hash_connect(struct inet_timewait_death_row *death_row,
> local_bh_disable();
> for (i = 1; i <= remaining; i++) {
> port = low + (i + offset) % remaining;
> + if (inet_is_reserved_local_port(port))
> + continue;
> head = &hinfo->bhash[inet_bhashfn(net, port,
> hinfo->bhash_size)];
> spin_lock(&head->lock);
> diff --git a/net/ipv4/sysctl_net_ipv4.c b/net/ipv4/sysctl_net_ipv4.c
> index 7e3712c..ce3597a 100644
> --- a/net/ipv4/sysctl_net_ipv4.c
> +++ b/net/ipv4/sysctl_net_ipv4.c
> @@ -298,6 +298,13 @@ static struct ctl_table ipv4_table[] = {
> .mode = 0644,
> .proc_handler = ipv4_local_port_range,
> },
> + {
> + .procname = "ip_local_reserved_ports",
> + .data = NULL, /* initialized in sysctl_ipv4_init */
> + .maxlen = 65536,
> + .mode = 0644,
> + .proc_handler = proc_dobitmap,
> + },
Isn't there an off-by-one here?
In patch 2/3, you use 0 to set the fist bit, then how about 65535 which
writes 65536th bit? This is beyond the range of port number.
> #ifdef CONFIG_IP_MULTICAST
> {
> .procname = "igmp_max_memberships",
> @@ -721,6 +728,16 @@ static __net_initdata struct pernet_operations ipv4_sysctl_ops = {
> static __init int sysctl_ipv4_init(void)
> {
> struct ctl_table_header *hdr;
> + struct ctl_table *i;
> +
> + for (i = ipv4_table; i->procname; i++) {
> + if (strcmp(i->procname, "ip_local_reserved_ports") == 0) {
> + i->data = sysctl_local_reserved_ports;
> + break;
> + }
> + }
> + if (!i->procname[0])
> + return -EINVAL;
>
> hdr = register_sysctl_paths(net_ipv4_ctl_path, ipv4_table);
> if (hdr == NULL)
> diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c
> index 608a544..bfd0a6a 100644
> --- a/net/ipv4/udp.c
> +++ b/net/ipv4/udp.c
> @@ -232,7 +232,8 @@ int udp_lib_get_port(struct sock *sk, unsigned short snum,
> */
> do {
> if (low <= snum && snum <= high &&
> - !test_bit(snum >> udptable->log, bitmap))
> + !test_bit(snum >> udptable->log, bitmap) &&
> + !inet_is_reserved_local_port(snum))
> goto found;
> snum += rand;
> } while (snum != first);
> diff --git a/net/sctp/socket.c b/net/sctp/socket.c
> index f6d1e59..1f839d0 100644
> --- a/net/sctp/socket.c
> +++ b/net/sctp/socket.c
> @@ -5432,6 +5432,8 @@ static long sctp_get_port_local(struct sock *sk, union sctp_addr *addr)
> rover++;
> if ((rover < low) || (rover > high))
> rover = low;
> + if (inet_is_reserved_local_port(rover))
> + continue;
> index = sctp_phashfn(rover);
> head = &sctp_port_hashtable[index];
> sctp_spin_lock(&head->lock);
^ permalink raw reply
* Re: [net-next PATCH v4 2/3] sysctl: add proc_dobitmap
From: Cong Wang @ 2010-02-16 9:12 UTC (permalink / raw)
To: Octavian Purdila
Cc: David Miller, Linux Kernel Network Developers,
Linux Kernel Developers, Eric W. Biederman
In-Reply-To: <1266271241-6293-3-git-send-email-opurdila@ixiacom.com>
Octavian Purdila wrote:
> The new function can be used to update bitmaps via /proc. Bits can be
> set by writing positive values in the file and cleared by writing
> negative values (e.g. 0 2 will set bits 1 and 3, -0 -2 will clear
> them). Reading will show only the set bits.
>
> Signed-off-by: Octavian Purdila <opurdila@ixiacom.com>
> Cc: WANG Cong <amwang@redhat.com>
> Cc: Eric W. Biederman <ebiederm@xmission.com>
> ---
> include/linux/sysctl.h | 2 +
> kernel/sysctl.c | 76 ++++++++++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 78 insertions(+), 0 deletions(-)
>
> diff --git a/include/linux/sysctl.h b/include/linux/sysctl.h
> index 9f236cd..ba89bf2 100644
> --- a/include/linux/sysctl.h
> +++ b/include/linux/sysctl.h
> @@ -985,6 +985,8 @@ extern int proc_doulongvec_minmax(struct ctl_table *, int,
> void __user *, size_t *, loff_t *);
> extern int proc_doulongvec_ms_jiffies_minmax(struct ctl_table *table, int,
> void __user *, size_t *, loff_t *);
> +extern int proc_dobitmap(struct ctl_table *, int,
> + void __user *, size_t *, loff_t *);
>
> /*
> * Register a set of sysctl names by calling register_sysctl_table
> diff --git a/kernel/sysctl.c b/kernel/sysctl.c
> index b0f9618..b8959f4 100644
> --- a/kernel/sysctl.c
> +++ b/kernel/sysctl.c
> @@ -2596,6 +2596,82 @@ static int proc_do_cad_pid(struct ctl_table *table, int write,
> return 0;
> }
>
> +/**
> + * proc_dobitmap - read/write from/to a bitmap
> + * @table: the sysctl table
> + * @write: %TRUE if this is a write to the sysctl file
> + * @buffer: the user buffer
> + * @lenp: the size of the user buffer
> + * @ppos: file position
> + * @ppos: the current position in the file
Duplicated.
I hope Eric can also comment on these two patches.
^ permalink raw reply
* Re: [net-next PATCH v4 1/3] sysctl: refactor integer handling proc code
From: Cong Wang @ 2010-02-16 8:41 UTC (permalink / raw)
To: Octavian Purdila
Cc: David Miller, Linux Kernel Network Developers,
Linux Kernel Developers, Eric W. Biederman
In-Reply-To: <1266271241-6293-2-git-send-email-opurdila@ixiacom.com>
Octavian Purdila wrote:
> As we are about to add another integer handling proc function a little
> bit of cleanup is in order: add a few helper functions to improve code
> readability and decrease code duplication.
>
> In the process a bug is fixed as well: if the user specifies a number
> with more then 20 digits it will be interpreted as two integers
> (e.g. 10000...13 will be interpreted as 100.... and 13).
>
> Signed-off-by: Octavian Purdila <opurdila@ixiacom.com>
> Cc: WANG Cong <amwang@redhat.com>
> Cc: Eric W. Biederman <ebiederm@xmission.com>
Some comments below.
> ---
> kernel/sysctl.c | 298 +++++++++++++++++++++++++++----------------------------
> 1 files changed, 144 insertions(+), 154 deletions(-)
>
> diff --git a/kernel/sysctl.c b/kernel/sysctl.c
> index 8a68b24..b0f9618 100644
> --- a/kernel/sysctl.c
> +++ b/kernel/sysctl.c
> @@ -2039,8 +2039,98 @@ int proc_dostring(struct ctl_table *table, int write,
> buffer, lenp, ppos);
> }
>
> +static int proc_skip_wspace(char __user **buf, size_t *size)
> +{
> + char c;
> +
> + while (*size) {
> + if (get_user(c, *buf))
> + return -EFAULT;
> + if (!isspace(c))
> + break;
> + (*size)--; (*buf)++;
> + }
> +
> + return 0;
> +}
In lib/string.c we have skip_spaces(), I think we can use it
here instead of inventing another one.
> +
> +#define TMPBUFLEN 22
> +static int proc_get_next_ulong(char __user **buf, size_t *size,
> + unsigned long *val, bool *neg)
> +{
> + int len;
> + char *p, tmp[TMPBUFLEN];
> + int err;
> +
> + err = proc_skip_wspace(buf, size);
> + if (err)
> + return err;
> + if (!*size)
> + return -EINVAL;
> +
> + len = *size;
> + if (len > TMPBUFLEN-1)
> + len = TMPBUFLEN-1;
> +
> + if (copy_from_user(tmp, *buf, len))
> + return -EFAULT;
> +
> + tmp[len] = 0;
> + p = tmp;
> + if (*p == '-' && *size > 1) {
> + *neg = 1;
> + p++;
> + } else
> + *neg = 0;
> + if (*p < '0' || *p > '9')
> + return -EINVAL;
isdigit().
> +
> + *val = simple_strtoul(p, &p, 0);
> +
> + len = p - tmp;
> + if (((len < *size) && *p && !isspace(*p)) ||
> + /* We don't know if the next char is whitespace thus we may accept
> + * invalid integers (e.g. 1234...a) or two integers instead of one
> + * (e.g. 123...1). So lets not allow such large numbers. */
> + len == TMPBUFLEN - 1)
> + return -EINVAL;
>
> -static int do_proc_dointvec_conv(int *negp, unsigned long *lvalp,
> + *buf += len; *size -= len;
> +
> + return 0;
> +}
> +
> +static int proc_put_ulong(char __user **buf, size_t *size, unsigned long val,
> + bool neg, bool first)
> +{
> + int len;
> + char tmp[TMPBUFLEN], *p = tmp;
> +
> + if (!first)
> + *p++ = '\t';
> + sprintf(p, "%s%lu", neg ? "-" : "", val);
> + len = strlen(tmp);
> + if (len > *size)
> + len = *size;
> + if (copy_to_user(*buf, tmp, len))
> + return -EFAULT;
> + *size -= len;
> + *buf += len;
> + return 0;
> +}
> +#undef TMPBUFLEN
> +
> +static int proc_put_newline(char __user **buf, size_t *size)
> +{
> + if (*size) {
> + if (put_user('\n', *buf))
> + return -EFAULT;
> + (*size)--, (*buf)++;
> + }
> + return 0;
> +}
> +
> +static int do_proc_dointvec_conv(bool *negp, unsigned long *lvalp,
> int *valp,
> int write, void *data)
> {
> @@ -2049,7 +2139,7 @@ static int do_proc_dointvec_conv(int *negp, unsigned long *lvalp,
> } else {
> int val = *valp;
> if (val < 0) {
> - *negp = -1;
> + *negp = 1;
> *lvalp = (unsigned long)-val;
> } else {
> *negp = 0;
> @@ -2060,19 +2150,15 @@ static int do_proc_dointvec_conv(int *negp, unsigned long *lvalp,
> }
>
> static int __do_proc_dointvec(void *tbl_data, struct ctl_table *table,
> - int write, void __user *buffer,
> + int write, void __user *_buffer,
> size_t *lenp, loff_t *ppos,
> - int (*conv)(int *negp, unsigned long *lvalp, int *valp,
> + int (*conv)(bool *negp, unsigned long *lvalp, int *valp,
> int write, void *data),
> void *data)
> {
> -#define TMPBUFLEN 21
> - int *i, vleft, first = 1, neg;
> - unsigned long lval;
> - size_t left, len;
> -
> - char buf[TMPBUFLEN], *p;
> - char __user *s = buffer;
> + int *i, vleft, first = 1, err = 0;
> + size_t left;
> + char __user *buffer = (char __user *) _buffer;
>
> if (!tbl_data || !table->maxlen || !*lenp ||
> (*ppos && !write)) {
> @@ -2088,88 +2174,39 @@ static int __do_proc_dointvec(void *tbl_data, struct ctl_table *table,
> conv = do_proc_dointvec_conv;
>
> for (; left && vleft--; i++, first=0) {
> - if (write) {
> - while (left) {
> - char c;
> - if (get_user(c, s))
> - return -EFAULT;
> - if (!isspace(c))
> - break;
> - left--;
> - s++;
> - }
> - if (!left)
> - break;
> - neg = 0;
> - len = left;
> - if (len > sizeof(buf) - 1)
> - len = sizeof(buf) - 1;
> - if (copy_from_user(buf, s, len))
> - return -EFAULT;
> - buf[len] = 0;
> - p = buf;
> - if (*p == '-' && left > 1) {
> - neg = 1;
> - p++;
> - }
> - if (*p < '0' || *p > '9')
> - break;
> -
> - lval = simple_strtoul(p, &p, 0);
> + unsigned long lval;
> + bool neg;
>
> - len = p-buf;
> - if ((len < left) && *p && !isspace(*p))
> + if (write) {
> + err = proc_get_next_ulong(&buffer, &left, &lval, &neg);
> + if (err)
> break;
> - s += len;
> - left -= len;
> -
> if (conv(&neg, &lval, i, 1, data))
> break;
> } else {
> - p = buf;
> - if (!first)
> - *p++ = '\t';
> -
> if (conv(&neg, &lval, i, 0, data))
> break;
> -
> - sprintf(p, "%s%lu", neg ? "-" : "", lval);
> - len = strlen(buf);
> - if (len > left)
> - len = left;
> - if(copy_to_user(s, buf, len))
> - return -EFAULT;
> - left -= len;
> - s += len;
> - }
> - }
> -
> - if (!write && !first && left) {
> - if(put_user('\n', s))
> - return -EFAULT;
> - left--, s++;
> - }
> - if (write) {
> - while (left) {
> - char c;
> - if (get_user(c, s++))
> - return -EFAULT;
> - if (!isspace(c))
> + err = proc_put_ulong(&buffer, &left, lval, neg, first);
> + if (err)
> break;
> - left--;
> }
> }
> - if (write && first)
> - return -EINVAL;
> +
> + if (!write && !first && left && !err)
> + err = proc_put_newline(&buffer, &left);
> + if (write && !err)
> + err = proc_skip_wspace(&buffer, &left);
> + if (err == -EFAULT /* do we really need to check for -EFAULT? */ ||
> + (write && first))
> + return err ? : -EINVAL;
The logic here seems messy, adding one or two goto's may help?
> *lenp -= left;
> *ppos += *lenp;
> return 0;
> -#undef TMPBUFLEN
> }
>
The rest looks fine.
Thanks!
^ permalink raw reply
* Re: Re: [PATCH 18/23 v3] mlx4_core: Managing common port filters by
From: Or Gerlitz @ 2010-02-16 6:53 UTC (permalink / raw)
To: Yevgeny Petrilin; +Cc: Roland Dreier, netdev, tziporet, liranl, Alex Rosenbaum
In-Reply-To: <4B72EB6B.9000708@mellanox.co.il>
On Wed, Feb 10, 2010 at 7:22 PM, Yevgeny Petrilin
<yevgenyp@mellanox.co.il> wrote:
> On Sunday -10,January,-28163 09:59 PM, Or Gerlitz [or.gerlitz@gmail.com] wrote:
> The multicast bloom filter is a false-positive filter per port.
> All multicast packets go through the Multicast tables mechanism where every PF/VF is registered for every multicast address
okay, so the bloom filter is going and exact match is coming, understood.
>> As for the false-positive vlan filter, I understand that this means further filtering has to be applied by some driver. Where you
>> thinking on the 802q driver? what about the case where a VF driver is assigned a vlan, does the mlx4 VF code does filtering?
> The deal with the VLAN filter is similar, it includes all VIDs that some PF/VF requested.
So for a given VID, a unicast packet carrying it will be dispatched to
all the PFs/VFs which
have registered this VID? I guess this isn't the case, so please elaborate.
> We plan to add a MAC+VLAN filter implementation, that would cancel the need in this filter.
I'm still not clear in what level you expect the software filtering to happen
Or.
^ permalink raw reply
* Re: Re: [PATCH 13/23 v3] mlx4: Unicast Loopback support
From: Or Gerlitz @ 2010-02-16 6:45 UTC (permalink / raw)
To: Yevgeny Petrilin; +Cc: Roland Dreier, netdev, liranl, Tziporet Koren
In-Reply-To: <4B72E7DF.3010206@mellanox.co.il>
Yevgeny Petrilin <yevgenyp@mellanox.co.il> wrote:
> Or Gerlitz [or.gerlitz@gmail.com] wrote:
>> I wasn't sure what is the use case here, isn't loopback handled by higher levels at the network stack?
> The use case is two VMs using the same physical adapter.
I am still not with you: are you referring to the case where each VM
is being served by a
different VF? in that case, the VF driver (mlx4_en) has no way to know
its a "loopback" packet,
and switching between VFs can be programmed to the PF by the PF driver
(modified mlx4_core).
If you are talking to the case both VMs are being served by the same
PCI function --> same NIC
then again, loopback is handled in higher level.
Is there a 3rd use case?
Or.
^ permalink raw reply
* Re: [PATCH] ipcomp: double free at ipcomp_destroy()
From: Herbert Xu @ 2010-02-16 6:00 UTC (permalink / raw)
To: Alexey Dobriyan; +Cc: davem, netdev
In-Reply-To: <20100215080846.GA18446@gondor.apana.org.au>
On Mon, Feb 15, 2010 at 04:08:46PM +0800, Herbert Xu wrote:
>
> Doh, I was looking at the buggy xfrm_state_clone path (which
> incidently needs to be fixed to use xfrm_state_put).
Here's a fix for that problem.
xfrm: Fix xfrm_state_clone leak
xfrm_state_clone calls kfree instead of xfrm_state_put to free
a failed state. Depending on the state of the failed state, it
can cause leaks to things like module references.
All states should be freed by xfrm_state_put past the point of
xfrm_init_state.
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
diff --git a/net/xfrm/xfrm_state.c b/net/xfrm/xfrm_state.c
index b36cc34..f445ea1 100644
--- a/net/xfrm/xfrm_state.c
+++ b/net/xfrm/xfrm_state.c
@@ -1102,7 +1102,7 @@ static struct xfrm_state *xfrm_state_clone(struct xfrm_state *orig, int *errp)
int err = -ENOMEM;
struct xfrm_state *x = xfrm_state_alloc(net);
if (!x)
- goto error;
+ goto out;
memcpy(&x->id, &orig->id, sizeof(x->id));
memcpy(&x->sel, &orig->sel, sizeof(x->sel));
@@ -1160,16 +1160,10 @@ static struct xfrm_state *xfrm_state_clone(struct xfrm_state *orig, int *errp)
return x;
error:
+ xfrm_state_put(x);
+out:
if (errp)
*errp = err;
- if (x) {
- kfree(x->aalg);
- kfree(x->ealg);
- kfree(x->calg);
- kfree(x->encap);
- kfree(x->coaddr);
- }
- kfree(x);
return NULL;
}
Cheers,
--
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply related
* Re: [PATCH 2/2] X25: Dont let x25_bind use addresses containing characters
From: David Miller @ 2010-02-16 5:55 UTC (permalink / raw)
To: andrew.hendry; +Cc: netdev, linux-kernel
In-Reply-To: <1266148845.4731.45.camel@ibex>
From: Andrew Hendry <andrew.hendry@gmail.com>
Date: Sun, 14 Feb 2010 23:00:45 +1100
> Addresses should be all digits.
> Stops x25_bind using addresses containing characters.
>
> Signed-off-by: Andrew Hendry <andrew.hendry@gmail.com>
Applied.
^ 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