Netdev List
 help / color / mirror / Atom feed
* Re: [PATCH net-next-2.6 5/6] be2net: Add be_rx_polls counter
From: David Miller @ 2009-12-02  9:11 UTC (permalink / raw)
  To: ajitk; +Cc: netdev
In-Reply-To: <20091130035712.GA15554@serverengines.com>

From: Ajit Khaparde <ajitk@serverengines.com>
Date: Mon, 30 Nov 2009 09:27:22 +0530

> Add be_rx_polls to count number of times NAPI called rx poll function.
> 
> Signed-off-by: Ajit Khaparde <ajitk@serverengines.com>

Applied.

^ permalink raw reply

* Re: [PATCH net-next-2.6 6/6] be2net: fix unnecessary access to hardware to get link, port info
From: David Miller @ 2009-12-02  9:11 UTC (permalink / raw)
  To: ajitk; +Cc: netdev
In-Reply-To: <20091130035736.GA15579@serverengines.com>

From: Ajit Khaparde <ajitk@serverengines.com>
Date: Mon, 30 Nov 2009 09:27:46 +0530

> Every time while doing ethtool->get_settings we are accessing
> the hardware to get link status and port information.
> This is not necessary.  We now use the cached copy for this info
> and update it when the link status changes.
> 
> From: Suresh R <sureshr@serverengines.com>
> 
> Signed-off-by: Ajit Khaparde <ajitk@serverengines.com>

Applied.

^ permalink raw reply

* Re: [PATCH net-next-2.6] ieee802154: merge cleanup
From: David Miller @ 2009-12-02  9:13 UTC (permalink / raw)
  To: eric.dumazet; +Cc: netdev
In-Reply-To: <4B14B222.8090203@gmail.com>

From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Tue, 01 Dec 2009 07:05:22 +0100

> [PATCH net-next-2.6] ieee802154: merge cleanup
> 
> A small cleanup after last net-2.6 merge into net-next-2.6
> 
> As we are going to free skb, no need to set skb->skb_iif or skb->dev
> 
> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>

Applied, thanks.

^ permalink raw reply

* Re: [PATCH] sch_htb: ix the deficit overflows
From: David Miller @ 2009-12-02  9:20 UTC (permalink / raw)
  To: jarkao2; +Cc: xiaosuo, hadi, netdev, martin.devera
In-Reply-To: <20091130111020.GA7114@ff.dom.local>

From: Jarek Poplawski <jarkao2@gmail.com>
Date: Mon, 30 Nov 2009 11:10:20 +0000

> On Mon, Nov 30, 2009 at 12:26:33PM +0800, Changli Gao wrote:
>> And
>> if we use IMQ to shape traffic, the skb will be defragmented by
>> conntrack, and its size will be larger than MTU.
> 
> IMQ is a very nice thing, but it's considered broken as well, so it
> can't be the reason for changing HTB.

If you don't like IMQ, fine.  Simply consider TSO and GSO as another
set of mechanisms that can introduce this condition.

Because we toss large SKBs all over the strack quite freely,
protections like those suggested by Changli make perfect sense.

We really don't have an MTU for packets within our stack any more.
The code, by default, need to be able to handle anything.

^ permalink raw reply

* Re: [PATCH 1/1] net: compat_sys_recvmmsg user timespec arg can be NULL
From: David Miller @ 2009-12-02  9:23 UTC (permalink / raw)
  To: acme; +Cc: netdev, jean-mickael.guerin, acme
In-Reply-To: <1259689936-27570-1-git-send-email-acme@infradead.org>

From: Arnaldo Carvalho de Melo <acme@infradead.org>
Date: Tue,  1 Dec 2009 15:52:16 -0200

> From: Jean-Mickael Guerin <jean-mickael.guerin@6wind.com>
> 
> We must test if user timespec is non-NULL before copying from userpace,
> same as sys_recvmmsg().
> 
> Commiter note: changed it so that we have just one branch.
> 
> Signed-off-by: Jean-Mickael Guerin <jean-mickael.guerin@6wind.com>
> Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>

Applied.

^ permalink raw reply

* Re: [PATCH 1/1] net: compat_mmsghdr must be used in sys_recvmmsg
From: David Miller @ 2009-12-02  9:23 UTC (permalink / raw)
  To: acme; +Cc: netdev, jean-mickael.guerin, acme
In-Reply-To: <1259693246-30341-1-git-send-email-acme@infradead.org>

From: Arnaldo Carvalho de Melo <acme@infradead.org>
Date: Tue,  1 Dec 2009 16:47:26 -0200

> From: Jean-Mickael Guerin <jean-mickael.guerin@6wind.com>
> 
> Both to traverse the entries and to set the msg_len field.
> 
> Commiter note: folded two patches and avoided one branch repeating the
> compat test.
> 
> Signed-off-by: Jean-Mickael Guerin <jean-mickael.guerin@6wind.com>
> Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>

Applied.

^ permalink raw reply

* Re: [PATCH] tcp: Fix a connect() race with timewait sockets
From: Eric Dumazet @ 2009-12-02  9:23 UTC (permalink / raw)
  To: David Miller; +Cc: kdakhane, netdev, netfilter, zbr
In-Reply-To: <20091202.005937.177088443.davem@davemloft.net>

David Miller a écrit :
> From: Eric Dumazet <eric.dumazet@gmail.com>
> Date: Tue, 01 Dec 2009 16:00:39 +0100
> 
>> [PATCH] tcp: Fix a connect() race with timewait sockets
> 
> This condition would only trigger if the timewait recycling sysctl is
> enabled.
> 
> It is off by default, and I can't find any mention in this bug report
> that it has been turned on.

Very true. I know nothing about context of the reporter, he didnt
answered to my queries.

Yes, if sysctl_tw_reuse is set, bug can triggers without any extra conditions.

But even if sysctl_tw_reuse is cleared, we might trigger the bug if
local port is bound to a value.

[User application called bind( port=XXX) before connect() ]


__inet_hash_connect() can indeed call check_established(... twp = NULL)

...
        head = &hinfo->bhash[inet_bhashfn(net, snum, hinfo->bhash_size)];
        tb  = inet_csk(sk)->icsk_bind_hash;
        spin_lock_bh(&head->lock);
        if (sk_head(&tb->owners) == sk && !sk->sk_bind_node.next) {
                hash(sk);
                spin_unlock_bh(&head->lock);
                return 0;
        } else {
                spin_unlock(&head->lock);
                /* No definite answer... Walk to established hash table */
                ret = check_established(death_row, sk, snum, NULL);         <<< HERE >>>
out:
                local_bh_enable();
                return ret;
        }



In this case, we call tcp_twsk_unique() with twp = NULL,
this bypass the sysctl_tcp_tw_reuse test.


int tcp_twsk_unique(struct sock *sk, struct sock *sktw, void *twp)
{
        const struct tcp_timewait_sock *tcptw = tcp_twsk(sktw);
        struct tcp_sock *tp = tcp_sk(sk);

        /* With PAWS, it is safe from the viewpoint
           of data integrity. Even without PAWS it is safe provided sequence
           spaces do not overlap i.e. at data rates <= 80Mbit/sec.

           Actually, the idea is close to VJ's one, only timestamp cache is
           held not per host, but per port pair and TW bucket is used as state
           holder.

           If TW bucket has been already destroyed we fall back to VJ's scheme
           and use initial timestamp retrieved from peer table.
         */
        if (tcptw->tw_ts_recent_stamp &&
<<HERE>>       (twp == NULL || (sysctl_tcp_tw_reuse &&
                             get_seconds() - tcptw->tw_ts_recent_stamp > 1))) {

^ permalink raw reply

* Re: [PATCH] net: Coding style corrections on Sealevel Systems 4021 driver.
From: David Miller @ 2009-12-02  9:26 UTC (permalink / raw)
  To: rudy.matela; +Cc: trivial, linux-kernel, netdev
In-Reply-To: <20091129024944.GB20636@oblivion>

From: Rudy Matela <rudy.matela@gmail.com>
Date: Sat, 28 Nov 2009 23:49:44 -0300

> Changed braces position on some statements.
> Inserted a space between an "if" keyword and a parenthesis.
> 
> Signed-off-by: Rudy Matela <rudy.matela@gmail.com>

Applied, thanks.

^ permalink raw reply

* Re: [PATCH] sch_htb: ix the deficit overflows
From: Jarek Poplawski @ 2009-12-02 10:32 UTC (permalink / raw)
  To: David Miller; +Cc: xiaosuo, hadi, netdev, martin.devera
In-Reply-To: <20091202.012017.39623676.davem@davemloft.net>

On Wed, Dec 02, 2009 at 01:20:17AM -0800, David Miller wrote:
> From: Jarek Poplawski <jarkao2@gmail.com>
> Date: Mon, 30 Nov 2009 11:10:20 +0000
> 
> > On Mon, Nov 30, 2009 at 12:26:33PM +0800, Changli Gao wrote:
> >> And
> >> if we use IMQ to shape traffic, the skb will be defragmented by
> >> conntrack, and its size will be larger than MTU.
> > 
> > IMQ is a very nice thing, but it's considered broken as well, so it
> > can't be the reason for changing HTB.
> 
> If you don't like IMQ, fine.  Simply consider TSO and GSO as another
> set of mechanisms that can introduce this condition.
> 
> Because we toss large SKBs all over the strack quite freely,
> protections like those suggested by Changli make perfect sense.
> 
> We really don't have an MTU for packets within our stack any more.
> The code, by default, need to be able to handle anything.

Alas, an MTU is still a crucial parameter for some schedulers. Anyway,
I hope Changli wasn't mislead to treat my private opinions in this or
any other thread as decisive. Otherwise, I'm sorry.

Jarek P.

^ permalink raw reply

* Re: [PATCH] tcp: Fix a connect() race with timewait sockets
From: Eric Dumazet @ 2009-12-02 10:33 UTC (permalink / raw)
  To: David Miller; +Cc: kdakhane, netdev, netfilter, zbr
In-Reply-To: <4B163226.50801@gmail.com>

Eric Dumazet a écrit :
> 
> But even if sysctl_tw_reuse is cleared, we might trigger the bug if
> local port is bound to a value.

Oh well, that's more subtle than that.

__inet_check_established() is called not only with bh disabled,
but also with a lock on bind list if twp != NULL.

However, if twp is NULL, lock is not held by caller.

[ Thats the final
  ret = check_established(death_row, sk, snum, NULL);
  in __inet_hash_connect()]

So triggering this bug with tw_reuse clear is tricky :

You need several threads, using sockets with REUSEADDR set,
and bind() to same address/port before connect() to same target.

We need another patch to correct this.

I wonder if always hold lock before calling check_established()
would be cleaner.


^ permalink raw reply

* Re: [PATCH] sch_htb: ix the deficit overflows
From: Martin Devera @ 2009-12-02 11:07 UTC (permalink / raw)
  To: Jarek Poplawski; +Cc: David Miller, xiaosuo, hadi, netdev
In-Reply-To: <20091202103204.GA7724@ff.dom.local>

Jarek Poplawski wrote:
>>
>> Because we toss large SKBs all over the strack quite freely,
>> protections like those suggested by Changli make perfect sense.
>>
>> We really don't have an MTU for packets within our stack any more.
>> The code, by default, need to be able to handle anything.
> 
> Alas, an MTU is still a crucial parameter for some schedulers. Anyway,
> I hope Changli wasn't mislead to treat my private opinions in this or
> any other thread as decisive. Otherwise, I'm sorry.

It seems that the new code is not slower when quantums are larger
than MTU's. Only when it is not the case, the new code introduces
loop which will iteratively increase deficit[x] until some packet
fits.
This can slow dequeue down. I'm not sure how much - but imagine
working setup with small quantums which works just now (only
sharing ratios are wrong).
After installing new kernel with the change the load could go
up and router could start  dropping packets.

While I don't believe the load will change so much, Changli
should measure this one IMHO.


^ permalink raw reply

* Re: [PATCH] tcp: Fix a connect() race with timewait sockets
From: Evgeniy Polyakov @ 2009-12-02 11:32 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: David Miller, kdakhane, netdev, netfilter
In-Reply-To: <4B164293.7070804@gmail.com>

On Wed, Dec 02, 2009 at 11:33:55AM +0100, Eric Dumazet (eric.dumazet@gmail.com) wrote:
> You need several threads, using sockets with REUSEADDR set,
> and bind() to same address/port before connect() to same target.
> 
> We need another patch to correct this.
> 
> I wonder if always hold lock before calling check_established()
> would be cleaner.

Isnt this a too big overhead?

-- 
	Evgeniy Polyakov

^ permalink raw reply

* Re: scp stalls mysteriously
From: Ilpo Järvinen @ 2009-12-02 12:59 UTC (permalink / raw)
  To: Frederic Leroy; +Cc: Netdev, Asdo, David Miller, Eric Dumazet, Herbert Xu
In-Reply-To: <20091202085925.472136e2@houba>

[-- Attachment #1: Type: TEXT/PLAIN, Size: 1192 bytes --]

On Wed, 2 Dec 2009, Frederic Leroy wrote:

> Le Tue, 1 Dec 2009 22:27:01 +0200 (EET),
> "Ilpo Järvinen" <ilpo.jarvinen@helsinki.fi> a écrit :
> 
> > On Tue, 1 Dec 2009, Frederic Leroy wrote:
> >
> > Thanks, I'll hopefully have some time tomorrow to go through all the
> > new ones.

Thanks for the logs. ...It is very much confirmed now that the connection 
suddently dies.

What is directly inferrable from the logs: Losses happen. RTO is triggered 
for the first time (for some reason also fast retransmit is missing and it 
most likely it should have preceeded RTO), however, the retransmission 
that RTO should make is never captured into the tcpdump. Then when RTO 
triggers for the second time, the connection silently dies immediately and 
consistently for some reason. ...From that point on resets are sent if the 
peer keeps trying.

> > Also,
> > I wonder if you have had some working (past) kernel version?
> 
> I tried a 2.6.31, and it seems to work. If it really work (connection
> don't stall everytime at the moment), I'll try to make a "git bisect"
> asap.

In one of the cases, also the sg end dies (the 4th case). I suppose that 
was running earlier kernel already?

-- 
 i.

^ permalink raw reply

* Re: [Patch] net: fix an array index overflow
From: Dan Carpenter @ 2009-12-02 13:24 UTC (permalink / raw)
  To: Amerigo Wang; +Cc: linux-kernel, netdev, David S. Miller
In-Reply-To: <20091201082901.4678.16688.sendpatchset@localhost.localdomain>

On Tue, Dec 01, 2009 at 03:26:02AM -0500, Amerigo Wang wrote:
> 
> Don't use the address of an out-of-boundary element.
> 
> Maybe this is not harmful at runtime, but it is still
> good to improve it.
> 
> Signed-off-by: WANG Cong <amwang@redhat.com>
> Cc: David S. Miller <davem@davemloft.net>
> 

It may be coincidence but my static checker smatch also complains 
about the code you modified.

It's the wrong idea to fix code to please a checker.  You end up
doing things like adding an extra "return -ENOTREACHED" to silence
warnings.  Then the next person who writes a checker has to figure
out how to seperate the unreachable code which was added to suppress
gcc warnings from bits which are unreachable because of typos.

Really any code that a human can read, a static checker should also
be able to read.  Computer programs are just state machines.  At 
the function level they are quite small state machines.  It's all
logic and math which computers are very good at.  So it should be 
fairly easy to fix the checker.  ;)

(The above paragraph is funnier if you knew how sucky smatch is).

regards,
dan carpenter

> ---
> diff --git a/net/ipv4/af_inet.c b/net/ipv4/af_inet.c
> index 57737b8..2669361 100644
> --- a/net/ipv4/af_inet.c
> +++ b/net/ipv4/af_inet.c
> @@ -1586,7 +1586,7 @@ static int __init inet_init(void)
>  #endif
>  
>  	/* Register the socket-side information for inet_create. */
> -	for (r = &inetsw[0]; r < &inetsw[SOCK_MAX]; ++r)
> +	for (r = &inetsw[0]; r <= &inetsw[SOCK_MAX-1]; ++r)
>  		INIT_LIST_HEAD(r);
>  
>  	for (q = inetsw_array; q < &inetsw_array[INETSW_ARRAY_LEN]; ++q)

^ permalink raw reply

* Re: [Bugme-new] [Bug 14685] New: 2.6.30 -> 2.6.31: very high number of RX dropped in bnx2
From: Krzysztof Olędzki @ 2009-12-02 13:37 UTC (permalink / raw)
  To: Michael Chan
  Cc: 'Andrew Morton', netdev@vger.kernel.org,
	bugzilla-daemon@bugzilla.kernel.org,
	bugme-daemon@bugzilla.kernel.org
In-Reply-To: <1259603093.14964.79.camel@nseg_linux_HP1.broadcom.com>

On 2009-11-30 18:44, Michael Chan wrote:
> On Mon, 2009-11-30 at 07:33 -0800, Krzysztof Oledzki wrote:
>> # ethtool -S eth0|egrep "(rx_bcast_packets|rx_mcast_packets|
>> rx_fw_discards)"
>>       rx_mcast_packets: 2806
>>       rx_bcast_packets: 790016
>>       rx_fw_discards: 790016
> 
> This tells me that all the dropped packets are occurring in the iSCSI
> ring.
> 
> We have recently enabled iSCSI in the kernel and you probably don't have
> the userspace components to handle the iSCSI ring.  All broadcast
> packets are also received in the iSCSI ring and they are all dropped in
> this case without the userspace components.  Networking is not affected.
> If you rmmod the bnx2i and cnic drivers, the drop counter should stop
> counting.
> 
> We have newer firmware that will not include dropped iSCSI packets in
> the networking counters.  It should appear in the 2.6.33 kernel
> time-frame.

Yep, it was the cnic driver. Now I feel silly. Thank you for you help 
and sorry for the noise. I going to close it with RESOLVED-DOCUMENTED.

Best regards,

				Krzysztof Olędzki

^ permalink raw reply

* tg3: not checking dma errors?
From: Johannes Berg @ 2009-12-02 14:01 UTC (permalink / raw)
  To: netdev; +Cc: Matt Carlson, Michael Chan

[-- Attachment #1: Type: text/plain, Size: 941 bytes --]

I'm having some trouble with tg3 under memory pressure when it's
receiving lots of data, and so far this is the only thing I could find
-- unfortunately I can't capture the debug information on the machine --
netconsole obviously doesn't work, there's no serial and the output
doesn't fit on the screen...

So I figured I'd look at the code, and now I think there could be a bug
with mappings:

tg3_alloc_rx_skb:
...
        mapping = pci_map_single(tp->pdev, skb->data, skb_size,
                                 PCI_DMA_FROMDEVICE);

        map->skb = skb;
        pci_unmap_addr_set(map, mapping, mapping);

        if (src_map != NULL)
                src_map->skb = NULL;

        desc->addr_hi = ((u64)mapping >> 32);
        desc->addr_lo = ((u64)mapping & 0xffffffff);

        return skb_size;



Shouldn't that check that 'mapping' is != DMA_ERROR_CODE?

Similarly in tg3_run_loopback()?

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

^ permalink raw reply

* Re: Key change for TCP_MD5
From: William Allen Simpson @ 2009-12-02 14:13 UTC (permalink / raw)
  To: Xiaochun Lu; +Cc: netdev, linux-kernel
In-Reply-To: <911b679e0912011627q568132e8x655f109c75030725@mail.gmail.com>

Xiaochun Lu wrote:
> know rfc4808 is targeting to solve this issue. Does there any plan to
> implement rfc4808 for linux?
> 
There's a replacement effort at:

   http://www.ietf.org/internet-drafts/draft-ietf-tcpm-tcp-auth-opt-08.txt
   http://www.ietf.org/internet-drafts/draft-ietf-tcpm-tcp-ao-crypto-01.txt

Probably going another round with them before writing code for Linux.

^ permalink raw reply

* Re: wanPMC-CxT1E1
From: Bob Beers @ 2009-12-02 14:57 UTC (permalink / raw)
  To: Greg KH; +Cc: netdev, Krzysztof Halasa
In-Reply-To: <20091119222551.GA18710@kroah.com>

On Thu, Nov 19, 2009 at 5:25 PM, Greg KH <greg@kroah.com> wrote:
> Anyway, I need a "Signed-off-by:" line for the patch so that I can apply
> it.  Also, who is the original author of the driver, any ideas?  We
> should at least credit them with this, right?
>
> Can you resend with a signed-off-by: line, and the other fixes all
> rolled up into one patch?

Yes, but probably not for a couple of weeks.  Other priorities
 have taken my available time.  Meanwhile, I will ask my OSS poc
 who to list as original author.

thanks,

-- 
-Bob Beers

^ permalink raw reply

* [PATCH net-next-2.6] tcp: connect() race with timewait reuse
From: Eric Dumazet @ 2009-12-02 15:08 UTC (permalink / raw)
  To: David Miller; +Cc: kdakhane, netdev, netfilter, zbr, Evgeniy Polyakov
In-Reply-To: <4B164293.7070804@gmail.com>

Eric Dumazet a écrit :
> Eric Dumazet a écrit :
>> But even if sysctl_tw_reuse is cleared, we might trigger the bug if
>> local port is bound to a value.
> 
> Oh well, that's more subtle than that.
> 
> __inet_check_established() is called not only with bh disabled,
> but also with a lock on bind list if twp != NULL.
> 
> However, if twp is NULL, lock is not held by caller.
> 
> [ Thats the final
>   ret = check_established(death_row, sk, snum, NULL);
>   in __inet_hash_connect()]
> 
> So triggering this bug with tw_reuse clear is tricky :
> 
> You need several threads, using sockets with REUSEADDR set,
> and bind() to same address/port before connect() to same target.
> 
> We need another patch to correct this.
> 

Here is a separate patch for this issue, cooked on top of net-next-2.6
for testing purposes, and public discussion.

Thanks

[PATCH net-next-2.6] tcp: connect() race with timewait reuse

Its currently possible that several threads issuing a connect() find the same
timewait socket and try to reuse it, leading to list corruptions.

Condition for bug is that these threads bound their socket on same address/port
of to be found timewait socket, and connected to same target. (SO_REUSEADDR needed)

To fix this problem, we could unhash timewait socket while holding ehash lock,
to make sure lookups/changes will be serialized. Only first one find the timewait
socket, other ones find the established socket and return an EADDRNOTAVAIL error.

Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
---
 include/net/inet_timewait_sock.h |    2 +
 net/ipv4/inet_hashtables.c       |    7 +++--
 net/ipv4/inet_timewait_sock.c    |   36 ++++++++++++++++++++---------
 net/ipv6/inet6_hashtables.c      |   12 +++++----
 4 files changed, 39 insertions(+), 18 deletions(-)

diff --git a/include/net/inet_timewait_sock.h b/include/net/inet_timewait_sock.h
index 773b10f..59c80a0 100644
--- a/include/net/inet_timewait_sock.h
+++ b/include/net/inet_timewait_sock.h
@@ -199,6 +199,8 @@ static inline __be32 inet_rcv_saddr(const struct sock *sk)
 
 extern void inet_twsk_put(struct inet_timewait_sock *tw);
 
+extern void inet_twsk_unhash(struct inet_timewait_sock *tw);
+
 extern struct inet_timewait_sock *inet_twsk_alloc(const struct sock *sk,
 						  const int state);
 
diff --git a/net/ipv4/inet_hashtables.c b/net/ipv4/inet_hashtables.c
index 94ef51a..143ddb4 100644
--- a/net/ipv4/inet_hashtables.c
+++ b/net/ipv4/inet_hashtables.c
@@ -318,20 +318,21 @@ unique:
 	sk->sk_hash = hash;
 	WARN_ON(!sk_unhashed(sk));
 	__sk_nulls_add_node_rcu(sk, &head->chain);
+	if (tw) {
+		inet_twsk_unhash(tw);
+		NET_INC_STATS_BH(net, LINUX_MIB_TIMEWAITRECYCLED);
+	}
 	spin_unlock(lock);
 	sock_prot_inuse_add(sock_net(sk), sk->sk_prot, 1);
 
 	if (twp) {
 		*twp = tw;
-		NET_INC_STATS_BH(net, LINUX_MIB_TIMEWAITRECYCLED);
 	} else if (tw) {
 		/* Silly. Should hash-dance instead... */
 		inet_twsk_deschedule(tw, death_row);
-		NET_INC_STATS_BH(net, LINUX_MIB_TIMEWAITRECYCLED);
 
 		inet_twsk_put(tw);
 	}
-
 	return 0;
 
 not_unique:
diff --git a/net/ipv4/inet_timewait_sock.c b/net/ipv4/inet_timewait_sock.c
index 1f5d508..680d09b 100644
--- a/net/ipv4/inet_timewait_sock.c
+++ b/net/ipv4/inet_timewait_sock.c
@@ -14,6 +14,21 @@
 #include <net/inet_timewait_sock.h>
 #include <net/ip.h>
 
+
+/*
+ * unhash a timewait socket from established hash
+ * lock must be hold by caller
+ */
+void inet_twsk_unhash(struct inet_timewait_sock *tw)
+{
+	if (hlist_nulls_unhashed(&tw->tw_node))
+		return;
+
+	hlist_nulls_del_rcu(&tw->tw_node);
+	sk_nulls_node_init(&tw->tw_node);
+	inet_twsk_put(tw);
+}
+
 /* Must be called with locally disabled BHs. */
 static void __inet_twsk_kill(struct inet_timewait_sock *tw,
 			     struct inet_hashinfo *hashinfo)
@@ -24,12 +39,9 @@ static void __inet_twsk_kill(struct inet_timewait_sock *tw,
 	spinlock_t *lock = inet_ehash_lockp(hashinfo, tw->tw_hash);
 
 	spin_lock(lock);
-	if (hlist_nulls_unhashed(&tw->tw_node)) {
-		spin_unlock(lock);
-		return;
-	}
-	hlist_nulls_del_rcu(&tw->tw_node);
-	sk_nulls_node_init(&tw->tw_node);
+
+	inet_twsk_unhash(tw);
+
 	spin_unlock(lock);
 
 	/* Disassociate with bind bucket. */
@@ -37,9 +49,11 @@ static void __inet_twsk_kill(struct inet_timewait_sock *tw,
 			hashinfo->bhash_size)];
 	spin_lock(&bhead->lock);
 	tb = tw->tw_tb;
-	__hlist_del(&tw->tw_bind_node);
-	tw->tw_tb = NULL;
-	inet_bind_bucket_destroy(hashinfo->bind_bucket_cachep, tb);
+	if (tb) {
+		__hlist_del(&tw->tw_bind_node);
+		tw->tw_tb = NULL;
+		inet_bind_bucket_destroy(hashinfo->bind_bucket_cachep, tb);
+	}
 	spin_unlock(&bhead->lock);
 #ifdef SOCK_REFCNT_DEBUG
 	if (atomic_read(&tw->tw_refcnt) != 1) {
@@ -47,7 +61,8 @@ static void __inet_twsk_kill(struct inet_timewait_sock *tw,
 		       tw->tw_prot->name, tw, atomic_read(&tw->tw_refcnt));
 	}
 #endif
-	inet_twsk_put(tw);
+	if (tb)
+		inet_twsk_put(tw);
 }
 
 static noinline void inet_twsk_free(struct inet_timewait_sock *tw)
@@ -92,6 +107,7 @@ void __inet_twsk_hashdance(struct inet_timewait_sock *tw, struct sock *sk,
 	tw->tw_tb = icsk->icsk_bind_hash;
 	WARN_ON(!icsk->icsk_bind_hash);
 	inet_twsk_add_bind_node(tw, &tw->tw_tb->owners);
+	atomic_inc(&tw->tw_refcnt);
 	spin_unlock(&bhead->lock);
 
 	spin_lock(lock);
diff --git a/net/ipv6/inet6_hashtables.c b/net/ipv6/inet6_hashtables.c
index 00c6a3e..3681c00 100644
--- a/net/ipv6/inet6_hashtables.c
+++ b/net/ipv6/inet6_hashtables.c
@@ -250,19 +250,21 @@ unique:
 	 * in hash table socket with a funny identity. */
 	inet->inet_num = lport;
 	inet->inet_sport = htons(lport);
+	sk->sk_hash = hash;
 	WARN_ON(!sk_unhashed(sk));
 	__sk_nulls_add_node_rcu(sk, &head->chain);
-	sk->sk_hash = hash;
+	if (tw) {
+		inet_twsk_unhash(tw);
+		NET_INC_STATS_BH(net, LINUX_MIB_TIMEWAITRECYCLED);
+	}
 	spin_unlock(lock);
 	sock_prot_inuse_add(sock_net(sk), sk->sk_prot, 1);
 
-	if (twp != NULL) {
+	if (twp) {
 		*twp = tw;
-		NET_INC_STATS_BH(net, LINUX_MIB_TIMEWAITRECYCLED);
-	} else if (tw != NULL) {
+	} else if (tw) {
 		/* Silly. Should hash-dance instead... */
 		inet_twsk_deschedule(tw, death_row);
-		NET_INC_STATS_BH(net, LINUX_MIB_TIMEWAITRECYCLED);
 
 		inet_twsk_put(tw);
 	}

^ permalink raw reply related

* [PATCH 0/3] iproute: fix tc ipt/xt module.
From: Andreas Henriksson @ 2009-12-02 15:10 UTC (permalink / raw)
  To: shemminger; +Cc: netdev

The xtables API has changed since the tc xt module was written.
This mini-series moves the legacy stuff out of the way (kept for
compatibility with older iptables versions) and starts
a new fresh xt module.

Hopefully the xtables api will be kept stable in the future.
If so, this new xt module should continue working since it only
uses exposed functions (unlike the old one which had an internal
header to access internal functions in xtables).

Andreas Henriksson (3):
  Keep the old tc/ipt/xt module for compatibility.
  Add new (iptables 1.4.5 compatible) tc/ipt/xt module.
  Slightly improve the configure script.

 configure     |   93 ++++++++++--
 tc/Makefile   |   13 ++-
 tc/m_xt.c     |  161 +++++----------------
 tc/m_xt_old.c |  433 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 557 insertions(+), 143 deletions(-)
 create mode 100644 tc/m_xt_old.c


^ permalink raw reply

* [PATCH 1/3] Keep the old tc/ipt/xt module for compatibility.
From: Andreas Henriksson @ 2009-12-02 15:11 UTC (permalink / raw)
  To: shemminger; +Cc: netdev
In-Reply-To: <cover.1259765732.git.andreas@fatal.se>

Move the file and rename the configure flags.
The file is being kept around for iptables < 1.4.5 compatibility.

Signed-off-by: Andreas Henriksson <andreas@fatal.se>
---
 configure     |    4 +-
 tc/Makefile   |   13 ++-
 tc/m_xt.c     |  433 ---------------------------------------------------------
 tc/m_xt_old.c |  433 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 444 insertions(+), 439 deletions(-)
 delete mode 100644 tc/m_xt.c
 create mode 100644 tc/m_xt_old.c

diff --git a/configure b/configure
index df40370..f3b79b2 100755
--- a/configure
+++ b/configure
@@ -53,7 +53,7 @@ gcc -I$INCLUDE $IPTC -o /tmp/ipttest /tmp/ipttest.c $IPTL -ldl >/dev/null 2>&1
 
 if [ $? -eq 0 ]
 then
-	echo "TC_CONFIG_XT:=y" >>Config
+	echo "TC_CONFIG_XT_OLD:=y" >>Config
 	echo "using xtables seems no need for internal.h"
 else
 	echo "failed test 2"
@@ -86,7 +86,7 @@ gcc -I$INCLUDE $IPTC -o /tmp/ipttest /tmp/ipttest.c $IPTL -ldl >/dev/null 2>&1
 if [ $? -eq 0 ]
 then
 	echo "using xtables instead of iptables (need for internal.h)"
-	echo "TC_CONFIG_XT_H:=y" >>Config
+	echo "TC_CONFIG_XT_OLD_H:=y" >>Config
 
 else
 	echo "failed test 3 using iptables"
diff --git a/tc/Makefile b/tc/Makefile
index 027055c..f3dd2b7 100644
--- a/tc/Makefile
+++ b/tc/Makefile
@@ -48,12 +48,17 @@ ifeq ($(TC_CONFIG_XT),y)
   TCMODULES += m_xt.o
   LDLIBS += -lxtables
 else
-  ifeq ($(TC_CONFIG_XT_H),y)
+  ifeq ($(TC_CONFIG_XT_OLD),y)
+    TCMODULES += m_xt_old.o
+    LDLIBS += -lxtables
+  else
+    ifeq ($(TC_CONFIG_XT_OLD_H),y)
 	CFLAGS += -DTC_CONFIG_XT_H
-	TCMODULES += m_xt.o
+	TCMODULES += m_xt_old.o
 	LDLIBS += -lxtables
-  else
-  TCMODULES += m_ipt.o
+    else
+      TCMODULES += m_ipt.o
+    endif
   endif
 endif
 
diff --git a/tc/m_xt.c b/tc/m_xt.c
deleted file mode 100644
index 0c7ec60..0000000
--- a/tc/m_xt.c
+++ /dev/null
@@ -1,433 +0,0 @@
-/*
- * m_xt.c	xtables based targets
- * 		utilities mostly ripped from iptables <duh, its the linux way>
- *
- *		This program is free software; you can distribute it and/or
- *		modify it under the terms of the GNU General Public License
- *		as published by the Free Software Foundation; either version
- *		2 of the License, or (at your option) any later version.
- *
- * Authors:  J Hadi Salim (hadi@cyberus.ca)
- */
-
-/*XXX: in the future (xtables 1.4.3?) get rid of everything tagged
- * as TC_CONFIG_XT_H */
-
-#include <syslog.h>
-#include <sys/socket.h>
-#include <netinet/in.h>
-#include <arpa/inet.h>
-#include <net/if.h>
-#include <linux/netfilter.h>
-#include <linux/netfilter_ipv4/ip_tables.h>
-#include <xtables.h>
-#include "utils.h"
-#include "tc_util.h"
-#include <linux/tc_act/tc_ipt.h>
-#include <stdio.h>
-#include <getopt.h>
-#include <errno.h>
-#include <string.h>
-#include <netdb.h>
-#include <stdlib.h>
-#include <ctype.h>
-#include <stdarg.h>
-#include <limits.h>
-#include <unistd.h>
-#include <fcntl.h>
-#include <sys/wait.h>
-#ifdef TC_CONFIG_XT_H
-#include "xt-internal.h"
-#endif
-
-static const char *pname = "tc-ipt";
-static const char *tname = "mangle";
-static const char *pversion = "0.2";
-
-static const char *ipthooks[] = {
-	"NF_IP_PRE_ROUTING",
-	"NF_IP_LOCAL_IN",
-	"NF_IP_FORWARD",
-	"NF_IP_LOCAL_OUT",
-	"NF_IP_POST_ROUTING",
-};
-
-static struct option original_opts[] = {
-	{"jump", 1, 0, 'j'},
-	{0, 0, 0, 0}
-};
-
-static struct option *opts = original_opts;
-static unsigned int global_option_offset = 0;
-char *lib_dir;
-const char *program_version = XTABLES_VERSION;
-const char *program_name = "tc-ipt";
-struct afinfo afinfo = {
-	.family         = AF_INET,
-	.libprefix      = "libxt_",
-	.ipproto        = IPPROTO_IP,
-	.kmod           = "ip_tables",
-	.so_rev_target  = IPT_SO_GET_REVISION_TARGET,
-};
-
-
-#define OPTION_OFFSET 256
-
-/*XXX: TC_CONFIG_XT_H */
-static void free_opts(struct option *local_opts)
-{
-	if (local_opts != original_opts) {
-		free(local_opts);
-		opts = original_opts;
-		global_option_offset = 0;
-	}
-}
-
-/*XXX: TC_CONFIG_XT_H */
-static struct option *
-merge_options(struct option *oldopts, const struct option *newopts,
-	      unsigned int *option_offset)
-{
-	struct option *merge;
-	unsigned int num_old, num_new, i;
-
-	for (num_old = 0; oldopts[num_old].name; num_old++) ;
-	for (num_new = 0; newopts[num_new].name; num_new++) ;
-
-	*option_offset = global_option_offset + OPTION_OFFSET;
-
-	merge = malloc(sizeof (struct option) * (num_new + num_old + 1));
-	memcpy(merge, oldopts, num_old * sizeof (struct option));
-	for (i = 0; i < num_new; i++) {
-		merge[num_old + i] = newopts[i];
-		merge[num_old + i].val += *option_offset;
-	}
-	memset(merge + num_old + num_new, 0, sizeof (struct option));
-
-	return merge;
-}
-
-
-/*XXX: TC_CONFIG_XT_H */
-#ifndef TRUE
-#define TRUE 1
-#endif
-#ifndef FALSE
-#define FALSE 0
-#endif
-
-/*XXX: TC_CONFIG_XT_H */
-int
-check_inverse(const char option[], int *invert, int *my_optind, int argc)
-{
-        if (option && strcmp(option, "!") == 0) {
-                if (*invert)
-                        exit_error(PARAMETER_PROBLEM,
-                                   "Multiple `!' flags not allowed");
-                *invert = TRUE;
-                if (my_optind != NULL) {
-                        ++*my_optind;
-                        if (argc && *my_optind > argc)
-                                exit_error(PARAMETER_PROBLEM,
-                                           "no argument following `!'");
-                }
-
-                return TRUE;
-        }
-        return FALSE;
-}
-
-/*XXX: TC_CONFIG_XT_H */
-void exit_error(enum exittype status, const char *msg, ...)
-{
-        va_list args;
-
-        va_start(args, msg);
-        fprintf(stderr, "%s v%s: ", pname, pversion);
-        vfprintf(stderr, msg, args);
-        va_end(args);
-        fprintf(stderr, "\n");
-        /* On error paths, make sure that we don't leak memory */
-        exit(status);
-}
-
-/*XXX: TC_CONFIG_XT_H */
-static void set_revision(char *name, u_int8_t revision)
-{
-	/* Old kernel sources don't have ".revision" field,
-	*  but we stole a byte from name. */
-	name[IPT_FUNCTION_MAXNAMELEN - 2] = '\0';
-	name[IPT_FUNCTION_MAXNAMELEN - 1] = revision;
-}
-
-/*
- * we may need to check for version mismatch
-*/
-int
-build_st(struct xtables_target *target, struct xt_entry_target *t)
-{
-
-	size_t size =
-		    XT_ALIGN(sizeof (struct xt_entry_target)) + target->size;
-
-	if (NULL == t) {
-		target->t = fw_calloc(1, size);
-		target->t->u.target_size = size;
-		strcpy(target->t->u.user.name, target->name);
-		set_revision(target->t->u.user.name, target->revision);
-
-		if (target->init != NULL)
-			target->init(target->t);
-	} else {
-		target->t = t;
-	}
-	return 0;
-
-}
-
-inline void set_lib_dir(void)
-{
-
-	lib_dir = getenv("XTABLES_LIBDIR");
-	if (!lib_dir) {
-		lib_dir = getenv("IPTABLES_LIB_DIR");
-		if (lib_dir)
-			fprintf(stderr, "using deprecated IPTABLES_LIB_DIR \n");
-	}
-	if (lib_dir == NULL)
-		lib_dir = XT_LIB_DIR;
-
-}
-
-static int parse_ipt(struct action_util *a,int *argc_p,
-		     char ***argv_p, int tca_id, struct nlmsghdr *n)
-{
-	struct xtables_target *m = NULL;
-	struct ipt_entry fw;
-	struct rtattr *tail;
-	int c;
-	int rargc = *argc_p;
-	char **argv = *argv_p;
-	int argc = 0, iargc = 0;
-	char k[16];
-	int res = -1;
-	int size = 0;
-	int iok = 0, ok = 0;
-	__u32 hook = 0, index = 0;
-	res = 0;
-
-	set_lib_dir();
-
-	{
-		int i;
-		for (i = 0; i < rargc; i++) {
-			if (NULL == argv[i] || 0 == strcmp(argv[i], "action")) {
-				break;
-			}
-		}
-		iargc = argc = i;
-	}
-
-	if (argc <= 2) {
-		fprintf(stderr,"bad arguements to ipt %d vs %d \n", argc, rargc);
-		return -1;
-	}
-
-	while (1) {
-		c = getopt_long(argc, argv, "j:", opts, NULL);
-		if (c == -1)
-			break;
-		switch (c) {
-		case 'j':
-			m = find_target(optarg, TRY_LOAD);
-			if (NULL != m) {
-
-				if (0 > build_st(m, NULL)) {
-					printf(" %s error \n", m->name);
-					return -1;
-				}
-				opts =
-				    merge_options(opts, m->extra_opts,
-						  &m->option_offset);
-			} else {
-				fprintf(stderr," failed to find target %s\n\n", optarg);
-				return -1;
-			}
-			ok++;
-			break;
-
-		default:
-			memset(&fw, 0, sizeof (fw));
-			if (m) {
-				m->parse(c - m->option_offset, argv, 0,
-					 &m->tflags, NULL, &m->t);
-			} else {
-				fprintf(stderr," failed to find target %s\n\n", optarg);
-				return -1;
-
-			}
-			ok++;
-			break;
-
-		}
-	}
-
-	if (iargc > optind) {
-		if (matches(argv[optind], "index") == 0) {
-			if (get_u32(&index, argv[optind + 1], 10)) {
-				fprintf(stderr, "Illegal \"index\"\n");
-				free_opts(opts);
-				return -1;
-			}
-			iok++;
-
-			optind += 2;
-		}
-	}
-
-	if (!ok && !iok) {
-		fprintf(stderr," ipt Parser BAD!! (%s)\n", *argv);
-		return -1;
-	}
-
-	/* check that we passed the correct parameters to the target */
-	if (m)
-		m->final_check(m->tflags);
-
-	{
-		struct tcmsg *t = NLMSG_DATA(n);
-		if (t->tcm_parent != TC_H_ROOT
-		    && t->tcm_parent == TC_H_MAJ(TC_H_INGRESS)) {
-			hook = NF_IP_PRE_ROUTING;
-		} else {
-			hook = NF_IP_POST_ROUTING;
-		}
-	}
-
-	tail = NLMSG_TAIL(n);
-	addattr_l(n, MAX_MSG, tca_id, NULL, 0);
-	fprintf(stdout, "tablename: %s hook: %s\n ", tname, ipthooks[hook]);
-	fprintf(stdout, "\ttarget: ");
-
-	if (m)
-		m->print(NULL, m->t, 0);
-	fprintf(stdout, " index %d\n", index);
-
-	if (strlen(tname) > 16) {
-		size = 16;
-		k[15] = 0;
-	} else {
-		size = 1 + strlen(tname);
-	}
-	strncpy(k, tname, size);
-
-	addattr_l(n, MAX_MSG, TCA_IPT_TABLE, k, size);
-	addattr_l(n, MAX_MSG, TCA_IPT_HOOK, &hook, 4);
-	addattr_l(n, MAX_MSG, TCA_IPT_INDEX, &index, 4);
-	if (m)
-		addattr_l(n, MAX_MSG, TCA_IPT_TARG, m->t, m->t->u.target_size);
-	tail->rta_len = (void *) NLMSG_TAIL(n) - (void *) tail;
-
-	argc -= optind;
-	argv += optind;
-	*argc_p = rargc - iargc;
-	*argv_p = argv;
-
-	optind = 0;
-	free_opts(opts);
-	/* Clear flags if target will be used again */
-        m->tflags=0;
-        m->used=0;
-	/* Free allocated memory */
-        if (m->t)
-            free(m->t);
-
-
-	return 0;
-
-}
-
-static int
-print_ipt(struct action_util *au,FILE * f, struct rtattr *arg)
-{
-	struct rtattr *tb[TCA_IPT_MAX + 1];
-	struct xt_entry_target *t = NULL;
-
-	if (arg == NULL)
-		return -1;
-
-	set_lib_dir();
-
-	parse_rtattr_nested(tb, TCA_IPT_MAX, arg);
-
-	if (tb[TCA_IPT_TABLE] == NULL) {
-		fprintf(f, "[NULL ipt table name ] assuming mangle ");
-	} else {
-		fprintf(f, "tablename: %s ",
-			(char *) RTA_DATA(tb[TCA_IPT_TABLE]));
-	}
-
-	if (tb[TCA_IPT_HOOK] == NULL) {
-		fprintf(f, "[NULL ipt hook name ]\n ");
-		return -1;
-	} else {
-		__u32 hook;
-		hook = *(__u32 *) RTA_DATA(tb[TCA_IPT_HOOK]);
-		fprintf(f, " hook: %s \n", ipthooks[hook]);
-	}
-
-	if (tb[TCA_IPT_TARG] == NULL) {
-		fprintf(f, "\t[NULL ipt target parameters ] \n");
-		return -1;
-	} else {
-		struct xtables_target *m = NULL;
-		t = RTA_DATA(tb[TCA_IPT_TARG]);
-		m = find_target(t->u.user.name, TRY_LOAD);
-		if (NULL != m) {
-			if (0 > build_st(m, t)) {
-				fprintf(stderr, " %s error \n", m->name);
-				return -1;
-			}
-
-			opts =
-			    merge_options(opts, m->extra_opts,
-					  &m->option_offset);
-		} else {
-			fprintf(stderr, " failed to find target %s\n\n",
-				t->u.user.name);
-			return -1;
-		}
-		fprintf(f, "\ttarget ");
-		m->print(NULL, m->t, 0);
-		if (tb[TCA_IPT_INDEX] == NULL) {
-			fprintf(f, " [NULL ipt target index ]\n");
-		} else {
-			__u32 index;
-			index = *(__u32 *) RTA_DATA(tb[TCA_IPT_INDEX]);
-			fprintf(f, " \n\tindex %d", index);
-		}
-
-		if (tb[TCA_IPT_CNT]) {
-			struct tc_cnt *c  = RTA_DATA(tb[TCA_IPT_CNT]);;
-			fprintf(f, " ref %d bind %d", c->refcnt, c->bindcnt);
-		}
-		if (show_stats) {
-			if (tb[TCA_IPT_TM]) {
-				struct tcf_t *tm = RTA_DATA(tb[TCA_IPT_TM]);
-				print_tm(f,tm);
-			}
-		}
-		fprintf(f, " \n");
-
-	}
-	free_opts(opts);
-
-	return 0;
-}
-
-struct action_util ipt_action_util = {
-        .id = "ipt",
-        .parse_aopt = parse_ipt,
-        .print_aopt = print_ipt,
-};
-
diff --git a/tc/m_xt_old.c b/tc/m_xt_old.c
new file mode 100644
index 0000000..0c7ec60
--- /dev/null
+++ b/tc/m_xt_old.c
@@ -0,0 +1,433 @@
+/*
+ * m_xt.c	xtables based targets
+ * 		utilities mostly ripped from iptables <duh, its the linux way>
+ *
+ *		This program is free software; you can distribute it and/or
+ *		modify it under the terms of the GNU General Public License
+ *		as published by the Free Software Foundation; either version
+ *		2 of the License, or (at your option) any later version.
+ *
+ * Authors:  J Hadi Salim (hadi@cyberus.ca)
+ */
+
+/*XXX: in the future (xtables 1.4.3?) get rid of everything tagged
+ * as TC_CONFIG_XT_H */
+
+#include <syslog.h>
+#include <sys/socket.h>
+#include <netinet/in.h>
+#include <arpa/inet.h>
+#include <net/if.h>
+#include <linux/netfilter.h>
+#include <linux/netfilter_ipv4/ip_tables.h>
+#include <xtables.h>
+#include "utils.h"
+#include "tc_util.h"
+#include <linux/tc_act/tc_ipt.h>
+#include <stdio.h>
+#include <getopt.h>
+#include <errno.h>
+#include <string.h>
+#include <netdb.h>
+#include <stdlib.h>
+#include <ctype.h>
+#include <stdarg.h>
+#include <limits.h>
+#include <unistd.h>
+#include <fcntl.h>
+#include <sys/wait.h>
+#ifdef TC_CONFIG_XT_H
+#include "xt-internal.h"
+#endif
+
+static const char *pname = "tc-ipt";
+static const char *tname = "mangle";
+static const char *pversion = "0.2";
+
+static const char *ipthooks[] = {
+	"NF_IP_PRE_ROUTING",
+	"NF_IP_LOCAL_IN",
+	"NF_IP_FORWARD",
+	"NF_IP_LOCAL_OUT",
+	"NF_IP_POST_ROUTING",
+};
+
+static struct option original_opts[] = {
+	{"jump", 1, 0, 'j'},
+	{0, 0, 0, 0}
+};
+
+static struct option *opts = original_opts;
+static unsigned int global_option_offset = 0;
+char *lib_dir;
+const char *program_version = XTABLES_VERSION;
+const char *program_name = "tc-ipt";
+struct afinfo afinfo = {
+	.family         = AF_INET,
+	.libprefix      = "libxt_",
+	.ipproto        = IPPROTO_IP,
+	.kmod           = "ip_tables",
+	.so_rev_target  = IPT_SO_GET_REVISION_TARGET,
+};
+
+
+#define OPTION_OFFSET 256
+
+/*XXX: TC_CONFIG_XT_H */
+static void free_opts(struct option *local_opts)
+{
+	if (local_opts != original_opts) {
+		free(local_opts);
+		opts = original_opts;
+		global_option_offset = 0;
+	}
+}
+
+/*XXX: TC_CONFIG_XT_H */
+static struct option *
+merge_options(struct option *oldopts, const struct option *newopts,
+	      unsigned int *option_offset)
+{
+	struct option *merge;
+	unsigned int num_old, num_new, i;
+
+	for (num_old = 0; oldopts[num_old].name; num_old++) ;
+	for (num_new = 0; newopts[num_new].name; num_new++) ;
+
+	*option_offset = global_option_offset + OPTION_OFFSET;
+
+	merge = malloc(sizeof (struct option) * (num_new + num_old + 1));
+	memcpy(merge, oldopts, num_old * sizeof (struct option));
+	for (i = 0; i < num_new; i++) {
+		merge[num_old + i] = newopts[i];
+		merge[num_old + i].val += *option_offset;
+	}
+	memset(merge + num_old + num_new, 0, sizeof (struct option));
+
+	return merge;
+}
+
+
+/*XXX: TC_CONFIG_XT_H */
+#ifndef TRUE
+#define TRUE 1
+#endif
+#ifndef FALSE
+#define FALSE 0
+#endif
+
+/*XXX: TC_CONFIG_XT_H */
+int
+check_inverse(const char option[], int *invert, int *my_optind, int argc)
+{
+        if (option && strcmp(option, "!") == 0) {
+                if (*invert)
+                        exit_error(PARAMETER_PROBLEM,
+                                   "Multiple `!' flags not allowed");
+                *invert = TRUE;
+                if (my_optind != NULL) {
+                        ++*my_optind;
+                        if (argc && *my_optind > argc)
+                                exit_error(PARAMETER_PROBLEM,
+                                           "no argument following `!'");
+                }
+
+                return TRUE;
+        }
+        return FALSE;
+}
+
+/*XXX: TC_CONFIG_XT_H */
+void exit_error(enum exittype status, const char *msg, ...)
+{
+        va_list args;
+
+        va_start(args, msg);
+        fprintf(stderr, "%s v%s: ", pname, pversion);
+        vfprintf(stderr, msg, args);
+        va_end(args);
+        fprintf(stderr, "\n");
+        /* On error paths, make sure that we don't leak memory */
+        exit(status);
+}
+
+/*XXX: TC_CONFIG_XT_H */
+static void set_revision(char *name, u_int8_t revision)
+{
+	/* Old kernel sources don't have ".revision" field,
+	*  but we stole a byte from name. */
+	name[IPT_FUNCTION_MAXNAMELEN - 2] = '\0';
+	name[IPT_FUNCTION_MAXNAMELEN - 1] = revision;
+}
+
+/*
+ * we may need to check for version mismatch
+*/
+int
+build_st(struct xtables_target *target, struct xt_entry_target *t)
+{
+
+	size_t size =
+		    XT_ALIGN(sizeof (struct xt_entry_target)) + target->size;
+
+	if (NULL == t) {
+		target->t = fw_calloc(1, size);
+		target->t->u.target_size = size;
+		strcpy(target->t->u.user.name, target->name);
+		set_revision(target->t->u.user.name, target->revision);
+
+		if (target->init != NULL)
+			target->init(target->t);
+	} else {
+		target->t = t;
+	}
+	return 0;
+
+}
+
+inline void set_lib_dir(void)
+{
+
+	lib_dir = getenv("XTABLES_LIBDIR");
+	if (!lib_dir) {
+		lib_dir = getenv("IPTABLES_LIB_DIR");
+		if (lib_dir)
+			fprintf(stderr, "using deprecated IPTABLES_LIB_DIR \n");
+	}
+	if (lib_dir == NULL)
+		lib_dir = XT_LIB_DIR;
+
+}
+
+static int parse_ipt(struct action_util *a,int *argc_p,
+		     char ***argv_p, int tca_id, struct nlmsghdr *n)
+{
+	struct xtables_target *m = NULL;
+	struct ipt_entry fw;
+	struct rtattr *tail;
+	int c;
+	int rargc = *argc_p;
+	char **argv = *argv_p;
+	int argc = 0, iargc = 0;
+	char k[16];
+	int res = -1;
+	int size = 0;
+	int iok = 0, ok = 0;
+	__u32 hook = 0, index = 0;
+	res = 0;
+
+	set_lib_dir();
+
+	{
+		int i;
+		for (i = 0; i < rargc; i++) {
+			if (NULL == argv[i] || 0 == strcmp(argv[i], "action")) {
+				break;
+			}
+		}
+		iargc = argc = i;
+	}
+
+	if (argc <= 2) {
+		fprintf(stderr,"bad arguements to ipt %d vs %d \n", argc, rargc);
+		return -1;
+	}
+
+	while (1) {
+		c = getopt_long(argc, argv, "j:", opts, NULL);
+		if (c == -1)
+			break;
+		switch (c) {
+		case 'j':
+			m = find_target(optarg, TRY_LOAD);
+			if (NULL != m) {
+
+				if (0 > build_st(m, NULL)) {
+					printf(" %s error \n", m->name);
+					return -1;
+				}
+				opts =
+				    merge_options(opts, m->extra_opts,
+						  &m->option_offset);
+			} else {
+				fprintf(stderr," failed to find target %s\n\n", optarg);
+				return -1;
+			}
+			ok++;
+			break;
+
+		default:
+			memset(&fw, 0, sizeof (fw));
+			if (m) {
+				m->parse(c - m->option_offset, argv, 0,
+					 &m->tflags, NULL, &m->t);
+			} else {
+				fprintf(stderr," failed to find target %s\n\n", optarg);
+				return -1;
+
+			}
+			ok++;
+			break;
+
+		}
+	}
+
+	if (iargc > optind) {
+		if (matches(argv[optind], "index") == 0) {
+			if (get_u32(&index, argv[optind + 1], 10)) {
+				fprintf(stderr, "Illegal \"index\"\n");
+				free_opts(opts);
+				return -1;
+			}
+			iok++;
+
+			optind += 2;
+		}
+	}
+
+	if (!ok && !iok) {
+		fprintf(stderr," ipt Parser BAD!! (%s)\n", *argv);
+		return -1;
+	}
+
+	/* check that we passed the correct parameters to the target */
+	if (m)
+		m->final_check(m->tflags);
+
+	{
+		struct tcmsg *t = NLMSG_DATA(n);
+		if (t->tcm_parent != TC_H_ROOT
+		    && t->tcm_parent == TC_H_MAJ(TC_H_INGRESS)) {
+			hook = NF_IP_PRE_ROUTING;
+		} else {
+			hook = NF_IP_POST_ROUTING;
+		}
+	}
+
+	tail = NLMSG_TAIL(n);
+	addattr_l(n, MAX_MSG, tca_id, NULL, 0);
+	fprintf(stdout, "tablename: %s hook: %s\n ", tname, ipthooks[hook]);
+	fprintf(stdout, "\ttarget: ");
+
+	if (m)
+		m->print(NULL, m->t, 0);
+	fprintf(stdout, " index %d\n", index);
+
+	if (strlen(tname) > 16) {
+		size = 16;
+		k[15] = 0;
+	} else {
+		size = 1 + strlen(tname);
+	}
+	strncpy(k, tname, size);
+
+	addattr_l(n, MAX_MSG, TCA_IPT_TABLE, k, size);
+	addattr_l(n, MAX_MSG, TCA_IPT_HOOK, &hook, 4);
+	addattr_l(n, MAX_MSG, TCA_IPT_INDEX, &index, 4);
+	if (m)
+		addattr_l(n, MAX_MSG, TCA_IPT_TARG, m->t, m->t->u.target_size);
+	tail->rta_len = (void *) NLMSG_TAIL(n) - (void *) tail;
+
+	argc -= optind;
+	argv += optind;
+	*argc_p = rargc - iargc;
+	*argv_p = argv;
+
+	optind = 0;
+	free_opts(opts);
+	/* Clear flags if target will be used again */
+        m->tflags=0;
+        m->used=0;
+	/* Free allocated memory */
+        if (m->t)
+            free(m->t);
+
+
+	return 0;
+
+}
+
+static int
+print_ipt(struct action_util *au,FILE * f, struct rtattr *arg)
+{
+	struct rtattr *tb[TCA_IPT_MAX + 1];
+	struct xt_entry_target *t = NULL;
+
+	if (arg == NULL)
+		return -1;
+
+	set_lib_dir();
+
+	parse_rtattr_nested(tb, TCA_IPT_MAX, arg);
+
+	if (tb[TCA_IPT_TABLE] == NULL) {
+		fprintf(f, "[NULL ipt table name ] assuming mangle ");
+	} else {
+		fprintf(f, "tablename: %s ",
+			(char *) RTA_DATA(tb[TCA_IPT_TABLE]));
+	}
+
+	if (tb[TCA_IPT_HOOK] == NULL) {
+		fprintf(f, "[NULL ipt hook name ]\n ");
+		return -1;
+	} else {
+		__u32 hook;
+		hook = *(__u32 *) RTA_DATA(tb[TCA_IPT_HOOK]);
+		fprintf(f, " hook: %s \n", ipthooks[hook]);
+	}
+
+	if (tb[TCA_IPT_TARG] == NULL) {
+		fprintf(f, "\t[NULL ipt target parameters ] \n");
+		return -1;
+	} else {
+		struct xtables_target *m = NULL;
+		t = RTA_DATA(tb[TCA_IPT_TARG]);
+		m = find_target(t->u.user.name, TRY_LOAD);
+		if (NULL != m) {
+			if (0 > build_st(m, t)) {
+				fprintf(stderr, " %s error \n", m->name);
+				return -1;
+			}
+
+			opts =
+			    merge_options(opts, m->extra_opts,
+					  &m->option_offset);
+		} else {
+			fprintf(stderr, " failed to find target %s\n\n",
+				t->u.user.name);
+			return -1;
+		}
+		fprintf(f, "\ttarget ");
+		m->print(NULL, m->t, 0);
+		if (tb[TCA_IPT_INDEX] == NULL) {
+			fprintf(f, " [NULL ipt target index ]\n");
+		} else {
+			__u32 index;
+			index = *(__u32 *) RTA_DATA(tb[TCA_IPT_INDEX]);
+			fprintf(f, " \n\tindex %d", index);
+		}
+
+		if (tb[TCA_IPT_CNT]) {
+			struct tc_cnt *c  = RTA_DATA(tb[TCA_IPT_CNT]);;
+			fprintf(f, " ref %d bind %d", c->refcnt, c->bindcnt);
+		}
+		if (show_stats) {
+			if (tb[TCA_IPT_TM]) {
+				struct tcf_t *tm = RTA_DATA(tb[TCA_IPT_TM]);
+				print_tm(f,tm);
+			}
+		}
+		fprintf(f, " \n");
+
+	}
+	free_opts(opts);
+
+	return 0;
+}
+
+struct action_util ipt_action_util = {
+        .id = "ipt",
+        .parse_aopt = parse_ipt,
+        .print_aopt = print_ipt,
+};
+
-- 
1.6.5.3


^ permalink raw reply related

* [PATCH 2/3] Add new (iptables 1.4.5 compatible) tc/ipt/xt module.
From: Andreas Henriksson @ 2009-12-02 15:11 UTC (permalink / raw)
  To: shemminger; +Cc: netdev
In-Reply-To: <cover.1259765732.git.andreas@fatal.se>

Add a new cleaned up m_xt.c based on m_xt_old.c
The new m_xt.c has been updated to use the new names and new api
that xtables exposes in iptables 1.4.5.
All the old internal api cruft has also been dropped.

Additionally, a configure script test is added to check for
the new xtables api and set the TC_CONFIG_XT flag in Config.
(tc/Makefile already handles this flag in previous commit.)

Signed-off-by: Andreas Henriksson <andreas@fatal.se>
---
 configure |   27 +++++
 tc/m_xt.c |  346 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 373 insertions(+), 0 deletions(-)
 create mode 100644 tc/m_xt.c

diff --git a/configure b/configure
index f3b79b2..4fda7cb 100755
--- a/configure
+++ b/configure
@@ -28,6 +28,33 @@ rm -f /tmp/atmtest.c /tmp/atmtest
 
 echo -n " IPT	"
 
+#check if we have xtables from iptables >= 1.4.5.
+cat >/tmp/ipttest.c <<EOF
+#include <xtables.h>
+#include <linux/netfilter.h>
+static struct xtables_globals test_globals = {
+	.option_offset = 0,
+	.program_name = "tc-ipt",
+	.program_version = XTABLES_VERSION,
+	.orig_opts = NULL,
+	.opts = NULL,
+	.exit_err = NULL,
+};
+
+int main(int argc, char **argv)
+{
+	xtables_init_all(&test_globals, NFPROTO_IPV4);
+	return 0;
+}
+
+EOF
+
+if gcc -I$INCLUDE $IPTC -o /tmp/ipttest /tmp/ipttest.c $IPTL -ldl -lxtables >/dev/null 2>&1
+then
+	echo "TC_CONFIG_XT:=y" >>Config
+	echo "using xtables instead of iptables"
+fi
+
 #check if we need dont our internal header ..
 cat >/tmp/ipttest.c <<EOF
 #include <xtables.h>
diff --git a/tc/m_xt.c b/tc/m_xt.c
new file mode 100644
index 0000000..3972d2d
--- /dev/null
+++ b/tc/m_xt.c
@@ -0,0 +1,346 @@
+/*
+ * m_xt.c	xtables based targets
+ * 		utilities mostly ripped from iptables <duh, its the linux way>
+ *
+ *		This program is free software; you can distribute it and/or
+ *		modify it under the terms of the GNU General Public License
+ *		as published by the Free Software Foundation; either version
+ *		2 of the License, or (at your option) any later version.
+ *
+ * Authors:  J Hadi Salim (hadi@cyberus.ca)
+ */
+
+#include <syslog.h>
+#include <sys/socket.h>
+#include <netinet/in.h>
+#include <arpa/inet.h>
+#include <net/if.h>
+#include <limits.h>
+#include <linux/netfilter.h>
+#include <linux/netfilter_ipv4/ip_tables.h>
+#include <xtables.h>
+#include "utils.h"
+#include "tc_util.h"
+#include <linux/tc_act/tc_ipt.h>
+#include <stdio.h>
+#include <dlfcn.h>
+#include <getopt.h>
+#include <errno.h>
+#include <string.h>
+#include <netdb.h>
+#include <stdlib.h>
+#include <ctype.h>
+#include <stdarg.h>
+#include <limits.h>
+#include <unistd.h>
+#include <fcntl.h>
+#include <sys/wait.h>
+#ifndef XT_LIB_DIR
+#       define XT_LIB_DIR "/lib/xtables"
+#endif
+
+static const char *tname = "mangle";
+
+char *lib_dir;
+
+static const char *ipthooks[] = {
+	"NF_IP_PRE_ROUTING",
+	"NF_IP_LOCAL_IN",
+	"NF_IP_FORWARD",
+	"NF_IP_LOCAL_OUT",
+	"NF_IP_POST_ROUTING",
+};
+
+static struct option original_opts[] = {
+	{
+		.name = "jump",
+		.has_arg = 1,
+		.val = 'j'
+	},
+	{0, 0, 0, 0}
+};
+
+static struct xtables_globals tcipt_globals = {
+	.option_offset = 0,
+	.program_name = "tc-ipt",
+	.program_version = "0.2",
+	.orig_opts = original_opts,
+	.opts = original_opts,
+	.exit_err = NULL,
+};
+
+/*
+ * we may need to check for version mismatch
+*/
+int
+build_st(struct xtables_target *target, struct xt_entry_target *t)
+{
+
+	size_t size =
+		    XT_ALIGN(sizeof (struct xt_entry_target)) + target->size;
+
+	if (NULL == t) {
+		target->t = xtables_calloc(1, size);
+		target->t->u.target_size = size;
+		strcpy(target->t->u.user.name, target->name);
+		xtables_set_revision(target->t->u.user.name, target->revision);
+
+		if (target->init != NULL)
+			target->init(target->t);
+	} else {
+		target->t = t;
+	}
+	return 0;
+
+}
+
+inline void set_lib_dir(void)
+{
+
+	lib_dir = getenv("XTABLES_LIBDIR");
+	if (!lib_dir) {
+		lib_dir = getenv("IPTABLES_LIB_DIR");
+		if (lib_dir)
+			fprintf(stderr, "using deprecated IPTABLES_LIB_DIR \n");
+	}
+	if (lib_dir == NULL)
+		lib_dir = XT_LIB_DIR;
+
+}
+
+static int parse_ipt(struct action_util *a,int *argc_p,
+		     char ***argv_p, int tca_id, struct nlmsghdr *n)
+{
+	struct xtables_target *m = NULL;
+	struct ipt_entry fw;
+	struct rtattr *tail;
+	int c;
+	int rargc = *argc_p;
+	char **argv = *argv_p;
+	int argc = 0, iargc = 0;
+	char k[16];
+	int res = -1;
+	int size = 0;
+	int iok = 0, ok = 0;
+	__u32 hook = 0, index = 0;
+	res = 0;
+
+	xtables_init_all(&tcipt_globals, NFPROTO_IPV4);
+	set_lib_dir();
+
+	{
+		int i;
+		for (i = 0; i < rargc; i++) {
+			if (NULL == argv[i] || 0 == strcmp(argv[i], "action")) {
+				break;
+			}
+		}
+		iargc = argc = i;
+	}
+
+	if (argc <= 2) {
+		fprintf(stderr,"bad arguements to ipt %d vs %d \n", argc, rargc);
+		return -1;
+	}
+
+	while (1) {
+		c = getopt_long(argc, argv, "j:", tcipt_globals.opts, NULL);
+		if (c == -1)
+			break;
+		switch (c) {
+		case 'j':
+			m = xtables_find_target(optarg, XTF_TRY_LOAD);
+			if (NULL != m) {
+
+				if (0 > build_st(m, NULL)) {
+					printf(" %s error \n", m->name);
+					return -1;
+				}
+				tcipt_globals.opts =
+				    xtables_merge_options(tcipt_globals.opts,
+				                          m->extra_opts,
+				                          &m->option_offset);
+			} else {
+				fprintf(stderr," failed to find target %s\n\n", optarg);
+				return -1;
+			}
+			ok++;
+			break;
+
+		default:
+			memset(&fw, 0, sizeof (fw));
+			if (m) {
+				m->parse(c - m->option_offset, argv, 0,
+					 &m->tflags, NULL, &m->t);
+			} else {
+				fprintf(stderr," failed to find target %s\n\n", optarg);
+				return -1;
+
+			}
+			ok++;
+			break;
+
+		}
+	}
+
+	if (iargc > optind) {
+		if (matches(argv[optind], "index") == 0) {
+			if (get_u32(&index, argv[optind + 1], 10)) {
+				fprintf(stderr, "Illegal \"index\"\n");
+				xtables_free_opts(1);
+				return -1;
+			}
+			iok++;
+
+			optind += 2;
+		}
+	}
+
+	if (!ok && !iok) {
+		fprintf(stderr," ipt Parser BAD!! (%s)\n", *argv);
+		return -1;
+	}
+
+	/* check that we passed the correct parameters to the target */
+	if (m && m->final_check)
+		m->final_check(m->tflags);
+
+	{
+		struct tcmsg *t = NLMSG_DATA(n);
+		if (t->tcm_parent != TC_H_ROOT
+		    && t->tcm_parent == TC_H_MAJ(TC_H_INGRESS)) {
+			hook = NF_IP_PRE_ROUTING;
+		} else {
+			hook = NF_IP_POST_ROUTING;
+		}
+	}
+
+	tail = NLMSG_TAIL(n);
+	addattr_l(n, MAX_MSG, tca_id, NULL, 0);
+	fprintf(stdout, "tablename: %s hook: %s\n ", tname, ipthooks[hook]);
+	fprintf(stdout, "\ttarget: ");
+
+	if (m)
+		m->print(NULL, m->t, 0);
+	fprintf(stdout, " index %d\n", index);
+
+	if (strlen(tname) > 16) {
+		size = 16;
+		k[15] = 0;
+	} else {
+		size = 1 + strlen(tname);
+	}
+	strncpy(k, tname, size);
+
+	addattr_l(n, MAX_MSG, TCA_IPT_TABLE, k, size);
+	addattr_l(n, MAX_MSG, TCA_IPT_HOOK, &hook, 4);
+	addattr_l(n, MAX_MSG, TCA_IPT_INDEX, &index, 4);
+	if (m)
+		addattr_l(n, MAX_MSG, TCA_IPT_TARG, m->t, m->t->u.target_size);
+	tail->rta_len = (void *) NLMSG_TAIL(n) - (void *) tail;
+
+	argc -= optind;
+	argv += optind;
+	*argc_p = rargc - iargc;
+	*argv_p = argv;
+
+	optind = 0;
+	xtables_free_opts(1);
+	/* Clear flags if target will be used again */
+        m->tflags=0;
+        m->used=0;
+	/* Free allocated memory */
+        if (m->t)
+            free(m->t);
+
+
+	return 0;
+
+}
+
+static int
+print_ipt(struct action_util *au,FILE * f, struct rtattr *arg)
+{
+	struct rtattr *tb[TCA_IPT_MAX + 1];
+	struct xt_entry_target *t = NULL;
+
+	if (arg == NULL)
+		return -1;
+
+	xtables_init_all(&tcipt_globals, NFPROTO_IPV4);
+	set_lib_dir();
+
+	parse_rtattr_nested(tb, TCA_IPT_MAX, arg);
+
+	if (tb[TCA_IPT_TABLE] == NULL) {
+		fprintf(f, "[NULL ipt table name ] assuming mangle ");
+	} else {
+		fprintf(f, "tablename: %s ",
+			(char *) RTA_DATA(tb[TCA_IPT_TABLE]));
+	}
+
+	if (tb[TCA_IPT_HOOK] == NULL) {
+		fprintf(f, "[NULL ipt hook name ]\n ");
+		return -1;
+	} else {
+		__u32 hook;
+		hook = *(__u32 *) RTA_DATA(tb[TCA_IPT_HOOK]);
+		fprintf(f, " hook: %s \n", ipthooks[hook]);
+	}
+
+	if (tb[TCA_IPT_TARG] == NULL) {
+		fprintf(f, "\t[NULL ipt target parameters ] \n");
+		return -1;
+	} else {
+		struct xtables_target *m = NULL;
+		t = RTA_DATA(tb[TCA_IPT_TARG]);
+		m = xtables_find_target(t->u.user.name, XTF_TRY_LOAD);
+		if (NULL != m) {
+			if (0 > build_st(m, t)) {
+				fprintf(stderr, " %s error \n", m->name);
+				return -1;
+			}
+
+			tcipt_globals.opts =
+			    xtables_merge_options(tcipt_globals.opts,
+			                          m->extra_opts,
+			                          &m->option_offset);
+		} else {
+			fprintf(stderr, " failed to find target %s\n\n",
+				t->u.user.name);
+			return -1;
+		}
+		fprintf(f, "\ttarget ");
+		m->print(NULL, m->t, 0);
+		if (tb[TCA_IPT_INDEX] == NULL) {
+			fprintf(f, " [NULL ipt target index ]\n");
+		} else {
+			__u32 index;
+			index = *(__u32 *) RTA_DATA(tb[TCA_IPT_INDEX]);
+			fprintf(f, " \n\tindex %d", index);
+		}
+
+		if (tb[TCA_IPT_CNT]) {
+			struct tc_cnt *c  = RTA_DATA(tb[TCA_IPT_CNT]);;
+			fprintf(f, " ref %d bind %d", c->refcnt, c->bindcnt);
+		}
+		if (show_stats) {
+			if (tb[TCA_IPT_TM]) {
+				struct tcf_t *tm = RTA_DATA(tb[TCA_IPT_TM]);
+				print_tm(f,tm);
+			}
+		}
+		fprintf(f, " \n");
+
+	}
+	xtables_free_opts(1);
+
+	return 0;
+}
+
+struct action_util ipt_action_util = {
+        .id = "ipt",
+        .parse_aopt = parse_ipt,
+        .print_aopt = print_ipt,
+};
+
-- 
1.6.5.3


^ permalink raw reply related

* [PATCH 3/3] Slightly improve the configure script.
From: Andreas Henriksson @ 2009-12-02 15:12 UTC (permalink / raw)
  To: shemminger; +Cc: netdev
In-Reply-To: <cover.1259765732.git.andreas@fatal.se>

Split up in functions. Make XT checks bail if previous XT check
was successful.

This result improves the output of the configure script to not indicate
using iptables only because the last test failed (when previous ones could
have already succeded).

Signed-off-by: Andreas Henriksson <andreas@fatal.se>
---
 configure |   66 +++++++++++++++++++++++++++++++++++++++++++++++-------------
 1 files changed, 51 insertions(+), 15 deletions(-)

diff --git a/configure b/configure
index 4fda7cb..a903bb0 100755
--- a/configure
+++ b/configure
@@ -3,11 +3,8 @@
 #
 INCLUDE=${1:-"$PWD/include"}
 
-echo "# Generated config based on" $INCLUDE >Config
-
-echo "TC schedulers"
-
-echo -n " ATM	"
+function check_atm
+{
 cat >/tmp/atmtest.c <<EOF
 #include <atm.h>
 int main(int argc, char **argv) {
@@ -25,9 +22,10 @@ else
     echo no
 fi
 rm -f /tmp/atmtest.c /tmp/atmtest
+}
 
-echo -n " IPT	"
-
+function check_xt
+{
 #check if we have xtables from iptables >= 1.4.5.
 cat >/tmp/ipttest.c <<EOF
 #include <xtables.h>
@@ -52,7 +50,17 @@ EOF
 if gcc -I$INCLUDE $IPTC -o /tmp/ipttest /tmp/ipttest.c $IPTL -ldl -lxtables >/dev/null 2>&1
 then
 	echo "TC_CONFIG_XT:=y" >>Config
-	echo "using xtables instead of iptables"
+	echo "using xtables"
+fi
+rm -f /tmp/ipttest.c /tmp/ipttest
+}
+
+function check_xt_old
+{
+# bail if previous XT checks has already succeded.
+if grep TC_CONFIG_XT Config > /dev/null
+then
+	return
 fi
 
 #check if we need dont our internal header ..
@@ -81,9 +89,17 @@ gcc -I$INCLUDE $IPTC -o /tmp/ipttest /tmp/ipttest.c $IPTL -ldl >/dev/null 2>&1
 if [ $? -eq 0 ]
 then
 	echo "TC_CONFIG_XT_OLD:=y" >>Config
-	echo "using xtables seems no need for internal.h"
-else
-	echo "failed test 2"
+	echo "using old xtables (no need for xt-internal.h)"
+fi
+rm -f /tmp/ipttest.c /tmp/ipttest
+}
+
+function check_xt_old_internal_h
+{
+# bail if previous XT checks has already succeded.
+if grep TC_CONFIG_XT Config > /dev/null
+then
+	return
 fi
 
 #check if we need our own internal.h
@@ -112,10 +128,30 @@ gcc -I$INCLUDE $IPTC -o /tmp/ipttest /tmp/ipttest.c $IPTL -ldl >/dev/null 2>&1
 
 if [ $? -eq 0 ]
 then
-	echo "using xtables instead of iptables (need for internal.h)"
+	echo "using old xtables with xt-internal.h"
 	echo "TC_CONFIG_XT_OLD_H:=y" >>Config
-
-else
-	echo "failed test 3 using iptables"
 fi
 rm -f /tmp/ipttest.c /tmp/ipttest
+}
+
+function check_ipt
+{
+	if ! grep TC_CONFIG_XT Config > /dev/null
+	then
+		echo "using iptables"
+	fi
+}
+
+echo "# Generated config based on" $INCLUDE >Config
+
+echo "TC schedulers"
+
+echo -n " ATM	"
+check_atm
+
+echo -n " IPT	"
+check_xt
+check_xt_old
+check_xt_old_internal_h
+check_ipt
+
-- 
1.6.5.3


^ permalink raw reply related

* Re: scp stalls mysteriously
From: Frederic Leroy @ 2009-12-02 15:44 UTC (permalink / raw)
  To: Ilpo Järvinen; +Cc: Netdev, Asdo, David Miller, Eric Dumazet, Herbert Xu
In-Reply-To: <alpine.DEB.2.00.0912021438030.20416@melkinpaasi.cs.helsinki.fi>

On Wed, Dec 02, 2009 at 02:59:20PM +0200, Ilpo Järvinen wrote:
> On Wed, 2 Dec 2009, Frederic Leroy wrote:
> 
> > Le Tue, 1 Dec 2009 22:27:01 +0200 (EET),
> > "Ilpo Järvinen" <ilpo.jarvinen@helsinki.fi> a écrit :
> What is directly inferrable from the logs: Losses happen. RTO is triggered 
> for the first time (for some reason also fast retransmit is missing and it 
> most likely it should have preceeded RTO), however, the retransmission 
> that RTO should make is never captured into the tcpdump. Then when RTO 
> triggers for the second time, the connection silently dies immediately and 
> consistently for some reason. ...From that point on resets are sent if the 
> peer keeps trying.

If you want, I can add printk when a rto is made, in order to compare with tcpdump.

> > > I wonder if you have had some working (past) kernel version?
> > I tried a 2.6.31, and it seems to work. If it really work (connection
> > don't stall everytime at the moment), I'll try to make a "git bisect"
> > asap.

I can confirm now that 2.6.31 works well.

I made a little change during the test : 
Instead of launching scp on sg, I launched it from houba.
The scp command return then  to bash with an error, without stalling.
Which appears to me logical because the connection is killed from houba point of view.
 
> In one of the cases, also the sg end dies (the 4th case). I suppose that 
> was running earlier kernel already?

This case was the one with tcp_frto=2 and tcp_timestamps=0 on houba.

I only rebooted sg two days ago. It still 2.6.32-rc5. Nothing had been changed on this host during all tests.
tcp_frto=2
tcp_timestamps=1

-- 
Frédéric Leroy

^ permalink raw reply

* Re: scp stalls mysteriously
From: Ilpo Järvinen @ 2009-12-02 16:05 UTC (permalink / raw)
  To: Frederic Leroy
  Cc: Netdev, Asdo, David Miller, Eric Dumazet, Herbert Xu,
	Damian Lukowski
In-Reply-To: <20091202154403.GB30730@sd-11162.dedibox.fr>

[-- Attachment #1: Type: TEXT/PLAIN, Size: 13452 bytes --]

On Wed, 2 Dec 2009, Frederic Leroy wrote:

> On Wed, Dec 02, 2009 at 02:59:20PM +0200, Ilpo Järvinen wrote:
> > On Wed, 2 Dec 2009, Frederic Leroy wrote:
> > 
> > > Le Tue, 1 Dec 2009 22:27:01 +0200 (EET),
> > > "Ilpo Järvinen" <ilpo.jarvinen@helsinki.fi> a écrit :
> > What is directly inferrable from the logs: Losses happen. RTO is triggered 
> > for the first time (for some reason also fast retransmit is missing and it 
> > most likely it should have preceeded RTO), however, the retransmission 
> > that RTO should make is never captured into the tcpdump. Then when RTO 
> > triggers for the second time, the connection silently dies immediately and 
> > consistently for some reason. ...From that point on resets are sent if the 
> > peer keeps trying.
> 
> If you want, I can add printk when a rto is made, in order to compare 
> with tcpdump. 

It's rather conclusive already that the first one did happen
(icsk_retransmit is increased in very few places and your proc/net/tcp 
shows increase of that to 1 every time). What happened on the second one 
is unknown though (icsk_retransmits==1 at that point of time), I suppose 
it takes some path in the RTO code which leads to death of the connection.

> > > > I wonder if you have had some working (past) kernel version?
> > > I tried a 2.6.31, and it seems to work. If it really work (connection
> > > don't stall everytime at the moment), I'll try to make a "git bisect"
> > > asap.
> 
> I can confirm now that 2.6.31 works well.
> 
> I made a little change during the test : 
> Instead of launching scp on sg, I launched it from houba.
> The scp command return then  to bash with an error, without stalling.
> Which appears to me logical because the connection is killed from houba point of view.
>  
> > In one of the cases, also the sg end dies (the 4th case). I suppose that 
> > was running earlier kernel already?
> 
> This case was the one with tcp_frto=2 and tcp_timestamps=0 on houba.

I suppose we're confused, I was refering to .4. case, did you perhaps mix 
that up with the latest set of tests which yields .8.?

> I only rebooted sg two days ago. It still 2.6.32-rc5. Nothing had been 
> changed on this host during all tests. 
> tcp_frto=2
> tcp_timestamps=1

...Ah so it was running the new kernel all the way along, my bad, I 
somehow was able to misread your kernel version numbers in the earlier 
mail :-).

IIRC your kernels Asdo were older than that, right?

In the recent work, the most suspicious things are the new timeout things, 
I'll read them through once I've some time (but so far I've not found 
anything wrong in them but I of course can miss something subtle).  
...I've added Damian as CC if he has some idea. If you want you can try 
with a trivial revert of that stuff, I've included a patch for that below.


-- 
 i.


--
[PATCH] Revert new RTO backoff stuff

---
 Documentation/networking/ip-sysctl.txt |   37 ++++++---------------
 include/net/tcp.h                      |   35 ---------------------
 net/ipv4/tcp_input.c                   |    5 ++-
 net/ipv4/tcp_ipv4.c                    |   53 +++++---------------------------
 net/ipv4/tcp_timer.c                   |   13 +++-----
 5 files changed, 27 insertions(+), 116 deletions(-)

diff --git a/Documentation/networking/ip-sysctl.txt b/Documentation/networking/ip-sysctl.txt
index fbe427a..da07602 100644
--- a/Documentation/networking/ip-sysctl.txt
+++ b/Documentation/networking/ip-sysctl.txt
@@ -311,12 +311,9 @@ tcp_no_metrics_save - BOOLEAN
 	connections.
 
 tcp_orphan_retries - INTEGER
-	This value influences the timeout of a locally closed TCP connection,
-	when RTO retransmissions remain unacknowledged.
-	See tcp_retries2 for more details.
-
-	The default value is 7.
-	If your machine is a loaded WEB server,
+	How may times to retry before killing TCP connection, closed
+	by our side. Default value 7 corresponds to ~50sec-16min
+	depending on RTO. If you machine is loaded WEB server,
 	you should think about lowering this value, such sockets
 	may consume significant resources. Cf. tcp_max_orphans.
 
@@ -330,28 +327,16 @@ tcp_retrans_collapse - BOOLEAN
 	certain TCP stacks.
 
 tcp_retries1 - INTEGER
-	This value influences the time, after which TCP decides, that
-	something is wrong due to unacknowledged RTO retransmissions,
-	and reports this suspicion to the network layer.
-	See tcp_retries2 for more details.
-
-	RFC 1122 recommends at least 3 retransmissions, which is the
-	default.
+	How many times to retry before deciding that something is wrong
+	and it is necessary to report this suspicion to network layer.
+	Minimal RFC value is 3, it is default, which corresponds
+	to ~3sec-8min depending on RTO.
 
 tcp_retries2 - INTEGER
-	This value influences the timeout of an alive TCP connection,
-	when RTO retransmissions remain unacknowledged.
-	Given a value of N, a hypothetical TCP connection following
-	exponential backoff with an initial RTO of TCP_RTO_MIN would
-	retransmit N times before killing the connection at the (N+1)th RTO.
-
-	The default value of 15 yields a hypothetical timeout of 924.6
-	seconds and is a lower bound for the effective timeout.
-	TCP will effectively time out at the first RTO which exceeds the
-	hypothetical timeout.
-
-	RFC 1122 recommends at least 100 seconds for the timeout,
-	which corresponds to a value of at least 8.
+	How may times to retry before killing alive TCP connection.
+	RFC1122 says that the limit should be longer than 100 sec.
+	It is too small number.	Default value 15 corresponds to ~13-30min
+	depending on RTO.
 
 tcp_rfc1337 - BOOLEAN
 	If set, the TCP stack behaves conforming to RFC1337. If unset,
diff --git a/include/net/tcp.h b/include/net/tcp.h
index 03a49c7..983367e 100644
--- a/include/net/tcp.h
+++ b/include/net/tcp.h
@@ -469,7 +469,6 @@ extern void __tcp_push_pending_frames(struct sock *sk, unsigned int cur_mss,
 				      int nonagle);
 extern int tcp_may_send_now(struct sock *sk);
 extern int tcp_retransmit_skb(struct sock *, struct sk_buff *);
-extern void tcp_retransmit_timer(struct sock *sk);
 extern void tcp_xmit_retransmit_queue(struct sock *);
 extern void tcp_simple_retransmit(struct sock *);
 extern int tcp_trim_head(struct sock *, struct sk_buff *, u32);
@@ -522,17 +521,6 @@ extern int tcp_mtu_to_mss(struct sock *sk, int pmtu);
 extern int tcp_mss_to_mtu(struct sock *sk, int mss);
 extern void tcp_mtup_init(struct sock *sk);
 
-static inline void tcp_bound_rto(const struct sock *sk)
-{
-	if (inet_csk(sk)->icsk_rto > TCP_RTO_MAX)
-		inet_csk(sk)->icsk_rto = TCP_RTO_MAX;
-}
-
-static inline u32 __tcp_set_rto(const struct tcp_sock *tp)
-{
-	return (tp->srtt >> 3) + tp->rttvar;
-}
-
 static inline void __tcp_fast_path_on(struct tcp_sock *tp, u32 snd_wnd)
 {
 	tp->pred_flags = htonl((tp->tcp_header_len << 26) |
@@ -1259,29 +1247,6 @@ static inline struct sk_buff *tcp_write_queue_prev(struct sock *sk, struct sk_bu
 #define tcp_for_write_queue_from_safe(skb, tmp, sk)			\
 	skb_queue_walk_from_safe(&(sk)->sk_write_queue, skb, tmp)
 
-/* This function calculates a "timeout" which is equivalent to the timeout of a
- * TCP connection after "boundary" unsucessful, exponentially backed-off
- * retransmissions with an initial RTO of TCP_RTO_MIN.
- */
-static inline bool retransmits_timed_out(const struct sock *sk,
-					 unsigned int boundary)
-{
-	unsigned int timeout, linear_backoff_thresh;
-
-	if (!inet_csk(sk)->icsk_retransmits)
-		return false;
-
-	linear_backoff_thresh = ilog2(TCP_RTO_MAX/TCP_RTO_MIN);
-
-	if (boundary <= linear_backoff_thresh)
-		timeout = ((2 << boundary) - 1) * TCP_RTO_MIN;
-	else
-		timeout = ((2 << linear_backoff_thresh) - 1) * TCP_RTO_MIN +
-			  (boundary - linear_backoff_thresh) * TCP_RTO_MAX;
-
-	return (tcp_time_stamp - tcp_sk(sk)->retrans_stamp) >= timeout;
-}
-
 static inline struct sk_buff *tcp_send_head(struct sock *sk)
 {
 	return sk->sk_send_head;
diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
index d86784b..6322e62 100644
--- a/net/ipv4/tcp_input.c
+++ b/net/ipv4/tcp_input.c
@@ -685,7 +685,7 @@ static inline void tcp_set_rto(struct sock *sk)
 	 *    is invisible. Actually, Linux-2.4 also generates erratic
 	 *    ACKs in some circumstances.
 	 */
-	inet_csk(sk)->icsk_rto = __tcp_set_rto(tp);
+	inet_csk(sk)->icsk_rto = (tp->srtt >> 3) + tp->rttvar;
 
 	/* 2. Fixups made earlier cannot be right.
 	 *    If we do not estimate RTO correctly without them,
@@ -696,7 +696,8 @@ static inline void tcp_set_rto(struct sock *sk)
 	/* NOTE: clamping at TCP_RTO_MIN is not required, current algo
 	 * guarantees that rto is higher.
 	 */
-	tcp_bound_rto(sk);
+	if (inet_csk(sk)->icsk_rto > TCP_RTO_MAX)
+		inet_csk(sk)->icsk_rto = TCP_RTO_MAX;
 }
 
 /* Save metrics learned by this TCP session.
diff --git a/net/ipv4/tcp_ipv4.c b/net/ipv4/tcp_ipv4.c
index 7cda24b..702ce88 100644
--- a/net/ipv4/tcp_ipv4.c
+++ b/net/ipv4/tcp_ipv4.c
@@ -328,29 +328,26 @@ static void do_pmtu_discovery(struct sock *sk, struct iphdr *iph, u32 mtu)
  *
  */
 
-void tcp_v4_err(struct sk_buff *icmp_skb, u32 info)
+void tcp_v4_err(struct sk_buff *skb, u32 info)
 {
-	struct iphdr *iph = (struct iphdr *)icmp_skb->data;
-	struct tcphdr *th = (struct tcphdr *)(icmp_skb->data + (iph->ihl << 2));
-	struct inet_connection_sock *icsk;
+	struct iphdr *iph = (struct iphdr *)skb->data;
+	struct tcphdr *th = (struct tcphdr *)(skb->data + (iph->ihl << 2));
 	struct tcp_sock *tp;
 	struct inet_sock *inet;
-	const int type = icmp_hdr(icmp_skb)->type;
-	const int code = icmp_hdr(icmp_skb)->code;
+	const int type = icmp_hdr(skb)->type;
+	const int code = icmp_hdr(skb)->code;
 	struct sock *sk;
-	struct sk_buff *skb;
 	__u32 seq;
-	__u32 remaining;
 	int err;
-	struct net *net = dev_net(icmp_skb->dev);
+	struct net *net = dev_net(skb->dev);
 
-	if (icmp_skb->len < (iph->ihl << 2) + 8) {
+	if (skb->len < (iph->ihl << 2) + 8) {
 		ICMP_INC_STATS_BH(net, ICMP_MIB_INERRORS);
 		return;
 	}
 
 	sk = inet_lookup(net, &tcp_hashinfo, iph->daddr, th->dest,
-			iph->saddr, th->source, inet_iif(icmp_skb));
+			iph->saddr, th->source, inet_iif(skb));
 	if (!sk) {
 		ICMP_INC_STATS_BH(net, ICMP_MIB_INERRORS);
 		return;
@@ -370,7 +367,6 @@ void tcp_v4_err(struct sk_buff *icmp_skb, u32 info)
 	if (sk->sk_state == TCP_CLOSE)
 		goto out;
 
-	icsk = inet_csk(sk);
 	tp = tcp_sk(sk);
 	seq = ntohl(th->seq);
 	if (sk->sk_state != TCP_LISTEN &&
@@ -397,39 +393,6 @@ void tcp_v4_err(struct sk_buff *icmp_skb, u32 info)
 		}
 
 		err = icmp_err_convert[code].errno;
-		/* check if icmp_skb allows revert of backoff
-		 * (see draft-zimmermann-tcp-lcd) */
-		if (code != ICMP_NET_UNREACH && code != ICMP_HOST_UNREACH)
-			break;
-		if (seq != tp->snd_una  || !icsk->icsk_retransmits ||
-		    !icsk->icsk_backoff)
-			break;
-
-		icsk->icsk_backoff--;
-		inet_csk(sk)->icsk_rto = __tcp_set_rto(tp) <<
-					 icsk->icsk_backoff;
-		tcp_bound_rto(sk);
-
-		skb = tcp_write_queue_head(sk);
-		BUG_ON(!skb);
-
-		remaining = icsk->icsk_rto - min(icsk->icsk_rto,
-				tcp_time_stamp - TCP_SKB_CB(skb)->when);
-
-		if (remaining) {
-			inet_csk_reset_xmit_timer(sk, ICSK_TIME_RETRANS,
-						  remaining, TCP_RTO_MAX);
-		} else if (sock_owned_by_user(sk)) {
-			/* RTO revert clocked out retransmission,
-			 * but socket is locked. Will defer. */
-			inet_csk_reset_xmit_timer(sk, ICSK_TIME_RETRANS,
-						  HZ/20, TCP_RTO_MAX);
-		} else {
-			/* RTO revert clocked out retransmission.
-			 * Will retransmit now */
-			tcp_retransmit_timer(sk);
-		}
-
 		break;
 	case ICMP_TIME_EXCEEDED:
 		err = EHOSTUNREACH;
diff --git a/net/ipv4/tcp_timer.c b/net/ipv4/tcp_timer.c
index cdb2ca7..c520fb6 100644
--- a/net/ipv4/tcp_timer.c
+++ b/net/ipv4/tcp_timer.c
@@ -137,14 +137,13 @@ static int tcp_write_timeout(struct sock *sk)
 {
 	struct inet_connection_sock *icsk = inet_csk(sk);
 	int retry_until;
-	bool do_reset;
 
 	if ((1 << sk->sk_state) & (TCPF_SYN_SENT | TCPF_SYN_RECV)) {
 		if (icsk->icsk_retransmits)
 			dst_negative_advice(&sk->sk_dst_cache);
 		retry_until = icsk->icsk_syn_retries ? : sysctl_tcp_syn_retries;
 	} else {
-		if (retransmits_timed_out(sk, sysctl_tcp_retries1)) {
+		if (icsk->icsk_retransmits >= sysctl_tcp_retries1) {
 			/* Black hole detection */
 			tcp_mtu_probing(icsk, sk);
 
@@ -156,15 +155,13 @@ static int tcp_write_timeout(struct sock *sk)
 			const int alive = (icsk->icsk_rto < TCP_RTO_MAX);
 
 			retry_until = tcp_orphan_retries(sk, alive);
-			do_reset = alive ||
-				   !retransmits_timed_out(sk, retry_until);
 
-			if (tcp_out_of_resources(sk, do_reset))
+			if (tcp_out_of_resources(sk, alive || icsk->icsk_retransmits < retry_until))
 				return 1;
 		}
 	}
 
-	if (retransmits_timed_out(sk, retry_until)) {
+	if (icsk->icsk_retransmits >= retry_until) {
 		/* Has it gone just too far? */
 		tcp_write_err(sk);
 		return 1;
@@ -282,7 +279,7 @@ static void tcp_probe_timer(struct sock *sk)
  *	The TCP retransmit timer.
  */
 
-void tcp_retransmit_timer(struct sock *sk)
+static void tcp_retransmit_timer(struct sock *sk)
 {
 	struct tcp_sock *tp = tcp_sk(sk);
 	struct inet_connection_sock *icsk = inet_csk(sk);
@@ -388,7 +385,7 @@ void tcp_retransmit_timer(struct sock *sk)
 out_reset_timer:
 	icsk->icsk_rto = min(icsk->icsk_rto << 1, TCP_RTO_MAX);
 	inet_csk_reset_xmit_timer(sk, ICSK_TIME_RETRANS, icsk->icsk_rto, TCP_RTO_MAX);
-	if (retransmits_timed_out(sk, sysctl_tcp_retries1 + 1))
+	if (icsk->icsk_retransmits > sysctl_tcp_retries1)
 		__sk_dst_reset(sk);
 
 out:;
-- 
1.5.6.3

^ permalink raw reply related


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