* Re: [PATCH 1/2] cxgb3 - T3C support update
From: Jeff Garzik @ 2007-12-07 20:02 UTC (permalink / raw)
To: Divy Le Ray; +Cc: netdev, linux-kernel, swise, wenxiong
In-Reply-To: <20071205181501.22081.97696.stgit@speedy5>
Divy Le Ray wrote:
> From: Divy Le Ray <divy@chelsio.com>
>
> Update GPIO mapping for T3C.
> Update xgmac for T3C support.
> Fix typo in mtu table.
>
> Signed-off-by: Divy Le Ray <divy@chelsio.com>
applied #upstream-fixes
^ permalink raw reply
* Re: [PATCH 1/8] bonding: Remove trailing NULs from sysfs interface.
From: Jeff Garzik @ 2007-12-07 20:02 UTC (permalink / raw)
To: Jay Vosburgh; +Cc: netdev, Wagner Ferenc
In-Reply-To: <11970132363756-git-send-email-fubar@us.ibm.com>
Jay Vosburgh wrote:
> From: Wagner Ferenc <wferi@niif.hu>
>
> From: Wagner Ferenc <wferi@niif.hu>
>
> Also remove trailing spaces from multivalued files.
>
> This fixes output like for example:
>
> $ od -c /sys/class/net/bond0/bonding/slaves
> 0000000 e t h - l e f t e t h - r i g
> 0000020 h t \n \0
> 0000025
>
> It mostly entails deleting '+1'-s after sprintf() calls: the return value
> of sprintf is the number of characters printed, without the closing NUL,
> ie. exactly what the sysfs interface requires. The three multivalue
> cases are different, because they also have to swallow back a trailing
> space.
>
> Signed-off-by: Ferenc Wagner <wferi@niif.hu>
> Acked-by: Jay Vosburgh <fubar@us.ibm.com>
> ---
> drivers/net/bonding/bond_sysfs.c | 66 +++++++++++++++++--------------------
> 1 files changed, 30 insertions(+), 36 deletions(-)
applied 1-8 to #upstream-fixes
Your script is duplicating the "From: " line twice
^ permalink raw reply
* [PATCH] XFRM: RFC4303 compliant auditing
From: Paul Moore @ 2007-12-07 19:57 UTC (permalink / raw)
To: netdev, linux-audit
NOTE: This really is an RFC patch, it compiles and boots but that is pretty
much all I can promise at this point. I'm posting this patch to gather
feedback from the audit crowd about the continued overloading of
the AUDIT_MAC_IPSEC_EVENT message type - continue to use it or create
a new audit message type? Of course any other comments people may have
are always welcome.
This patch adds a number of new IPsec audit events to meet the auditing
requirements of RFC4303. This includes audit hooks for the following events:
* Could not find a valid SA [sections 2.1, 3.4.2]
. xfrm_audit_state_notfound()
. xfrm_audit_state_notfound_simple()
* Sequence number overflow [section 3.3.3]
. xfrm_audit_state_replay_overflow()
* Replayed packet [section 3.4.3]
. xfrm_audit_state_replay()
* Integrity check failure [sections 3.4.4.1, 3.4.4.2]
. xfrm_audit_state_icvfail()
While RFC4304 deals only with ESP most of the changes in this patch apply to
IPsec in general, i.e. both AH and ESP. The one case, integrity check
failure, where ESP specific code had to be modified the same was done to the
AH code for the sake of consistency.
---
include/net/xfrm.h | 14 ++++
net/ipv4/ah4.c | 1
net/ipv4/esp4.c | 1
net/ipv4/xfrm4_input.c | 6 +-
net/ipv6/ah6.c | 1
net/ipv6/esp6.c | 1
net/ipv6/xfrm6_input.c | 10 ++-
net/xfrm/xfrm_output.c | 2 +
net/xfrm/xfrm_state.c | 155 ++++++++++++++++++++++++++++++++++++++++++++++--
9 files changed, 177 insertions(+), 14 deletions(-)
diff --git a/include/net/xfrm.h b/include/net/xfrm.h
index c02e230..85ce8c1 100644
--- a/include/net/xfrm.h
+++ b/include/net/xfrm.h
@@ -492,11 +492,22 @@ extern void xfrm_audit_state_add(struct xfrm_state *x, int result,
u32 auid, u32 secid);
extern void xfrm_audit_state_delete(struct xfrm_state *x, int result,
u32 auid, u32 secid);
+extern void xfrm_audit_state_replay_overflow(struct xfrm_state *x,
+ struct sk_buff *skb);
+extern void xfrm_audit_state_notfound_simple(struct sk_buff *skb, u16 family);
+extern void xfrm_audit_state_notfound(struct sk_buff *skb, u16 family,
+ __be32 net_spi, __be32 net_seq);
+extern void xfrm_audit_state_icvfail(struct xfrm_state *x,
+ struct sk_buff *skb, u8 proto);
#else
#define xfrm_audit_policy_add(x, r, a, s) do { ; } while (0)
#define xfrm_audit_policy_delete(x, r, a, s) do { ; } while (0)
#define xfrm_audit_state_add(x, r, a, s) do { ; } while (0)
#define xfrm_audit_state_delete(x, r, a, s) do { ; } while (0)
+#define xfrm_audit_state_replay_overflow(x, s) do { ; } while (0)
+#define xfrm_audit_state_notfound_simple(s, f) do { ; } while (0)
+#define xfrm_audit_state_notfound(s, f, sp, sq) do { ; } while (0)
+#define xfrm_audit_state_icvfail(x, s, p) do { ; } while (0)
#endif /* CONFIG_AUDITSYSCALL */
static inline void xfrm_pol_hold(struct xfrm_policy *policy)
@@ -1045,7 +1056,8 @@ extern int xfrm_state_delete(struct xfrm_state *x);
extern int xfrm_state_flush(u8 proto, struct xfrm_audit *audit_info);
extern void xfrm_sad_getinfo(struct xfrmk_sadinfo *si);
extern void xfrm_spd_getinfo(struct xfrmk_spdinfo *si);
-extern int xfrm_replay_check(struct xfrm_state *x, __be32 seq);
+extern int xfrm_replay_check(struct xfrm_state *x,
+ struct sk_buff *skb, __be32 seq);
extern void xfrm_replay_advance(struct xfrm_state *x, __be32 seq);
extern void xfrm_replay_notify(struct xfrm_state *x, int event);
extern int xfrm_state_mtu(struct xfrm_state *x, int mtu);
diff --git a/net/ipv4/ah4.c b/net/ipv4/ah4.c
index 5fc346d..8eb19c9 100644
--- a/net/ipv4/ah4.c
+++ b/net/ipv4/ah4.c
@@ -180,6 +180,7 @@ static int ah_input(struct xfrm_state *x, struct sk_buff *skb)
err = -EINVAL;
if (memcmp(ahp->work_icv, auth_data, ahp->icv_trunc_len)) {
x->stats.integrity_failed++;
+ xfrm_audit_state_icvfail(x, skb, IPPROTO_AH);
goto out;
}
}
diff --git a/net/ipv4/esp4.c b/net/ipv4/esp4.c
index c31bccb..00ec285 100644
--- a/net/ipv4/esp4.c
+++ b/net/ipv4/esp4.c
@@ -183,6 +183,7 @@ static int esp_input(struct xfrm_state *x, struct sk_buff *skb)
if (unlikely(memcmp(esp->auth.work_icv, sum, alen))) {
x->stats.integrity_failed++;
+ xfrm_audit_state_icvfail(x, skb, IPPROTO_ESP);
goto out;
}
}
diff --git a/net/ipv4/xfrm4_input.c b/net/ipv4/xfrm4_input.c
index 5e95c8a..6d7be5e 100644
--- a/net/ipv4/xfrm4_input.c
+++ b/net/ipv4/xfrm4_input.c
@@ -56,8 +56,10 @@ int xfrm4_rcv_encap(struct sk_buff *skb, int nexthdr, __be32 spi,
x = xfrm_state_lookup((xfrm_address_t *)&iph->daddr, spi,
nexthdr, AF_INET);
- if (x == NULL)
+ if (x == NULL) {
+ xfrm_audit_state_notfound(skb, AF_INET, spi, seq);
goto drop;
+ }
spin_lock(&x->lock);
if (unlikely(x->km.state != XFRM_STATE_VALID))
@@ -66,7 +68,7 @@ int xfrm4_rcv_encap(struct sk_buff *skb, int nexthdr, __be32 spi,
if ((x->encap ? x->encap->encap_type : 0) != encap_type)
goto drop_unlock;
- if (x->props.replay_window && xfrm_replay_check(x, seq))
+ if (x->props.replay_window && xfrm_replay_check(x, skb, seq))
goto drop_unlock;
if (xfrm_state_check_expire(x))
diff --git a/net/ipv6/ah6.c b/net/ipv6/ah6.c
index 4eaf550..b7d2e19 100644
--- a/net/ipv6/ah6.c
+++ b/net/ipv6/ah6.c
@@ -383,6 +383,7 @@ static int ah6_input(struct xfrm_state *x, struct sk_buff *skb)
if (memcmp(ahp->work_icv, auth_data, ahp->icv_trunc_len)) {
LIMIT_NETDEBUG(KERN_WARNING "ipsec ah authentication error\n");
x->stats.integrity_failed++;
+ xfrm_audit_state_icvfail(x, skb, IPPROTO_AH);
goto free_out;
}
}
diff --git a/net/ipv6/esp6.c b/net/ipv6/esp6.c
index 7db66f1..d56db2b 100644
--- a/net/ipv6/esp6.c
+++ b/net/ipv6/esp6.c
@@ -178,6 +178,7 @@ static int esp6_input(struct xfrm_state *x, struct sk_buff *skb)
if (unlikely(memcmp(esp->auth.work_icv, sum, alen))) {
x->stats.integrity_failed++;
+ xfrm_audit_state_icvfail(x, skb, IPPROTO_ESP);
ret = -EINVAL;
goto out;
}
diff --git a/net/ipv6/xfrm6_input.c b/net/ipv6/xfrm6_input.c
index 5157837..28c1e1b 100644
--- a/net/ipv6/xfrm6_input.c
+++ b/net/ipv6/xfrm6_input.c
@@ -40,13 +40,15 @@ int xfrm6_rcv_spi(struct sk_buff *skb, int nexthdr, __be32 spi)
x = xfrm_state_lookup((xfrm_address_t *)&iph->daddr, spi,
nexthdr, AF_INET6);
- if (x == NULL)
+ if (x == NULL) {
+ xfrm_audit_state_notfound(skb, AF_INET6, spi, seq);
goto drop;
+ }
spin_lock(&x->lock);
if (unlikely(x->km.state != XFRM_STATE_VALID))
goto drop_unlock;
- if (x->props.replay_window && xfrm_replay_check(x, seq))
+ if (x->props.replay_window && xfrm_replay_check(x, skb, seq))
goto drop_unlock;
if (xfrm_state_check_expire(x))
@@ -217,8 +219,10 @@ int xfrm6_input_addr(struct sk_buff *skb, xfrm_address_t *daddr,
break;
}
- if (!xfrm_vec_one)
+ if (!xfrm_vec_one) {
+ xfrm_audit_state_notfound_simple(skb, AF_INET6);
goto drop;
+ }
/* Allocate new secpath or COW existing one. */
if (!skb->sp || atomic_read(&skb->sp->refcnt) != 1) {
diff --git a/net/xfrm/xfrm_output.c b/net/xfrm/xfrm_output.c
index f4bfd6c..14ce897 100644
--- a/net/xfrm/xfrm_output.c
+++ b/net/xfrm/xfrm_output.c
@@ -59,6 +59,8 @@ int xfrm_output(struct sk_buff *skb)
if (x->type->flags & XFRM_TYPE_REPLAY_PROT) {
XFRM_SKB_CB(skb)->seq = ++x->replay.oseq;
+ if (unlikely(x->replay.oseq == 0))
+ xfrm_audit_state_replay_overflow(x, skb);
if (xfrm_aevent_is_on())
xfrm_replay_notify(x, XFRM_REPLAY_UPDATE);
}
diff --git a/net/xfrm/xfrm_state.c b/net/xfrm/xfrm_state.c
index b291a82..cfef149 100644
--- a/net/xfrm/xfrm_state.c
+++ b/net/xfrm/xfrm_state.c
@@ -61,6 +61,13 @@ static unsigned int xfrm_state_genid;
static struct xfrm_state_afinfo *xfrm_state_get_afinfo(unsigned int family);
static void xfrm_state_put_afinfo(struct xfrm_state_afinfo *afinfo);
+#ifdef CONFIG_AUDITSYSCALL
+static void xfrm_audit_state_replay(struct xfrm_state *x,
+ struct sk_buff *skb, __be32 net_seq);
+#else
+#define xfrm_audit_state_replay(x, s, sq) do { ; } while (0)
+#endif /* CONFIG_AUDITSYSCALL */
+
static inline unsigned int xfrm_dst_hash(xfrm_address_t *daddr,
xfrm_address_t *saddr,
u32 reqid,
@@ -1610,13 +1617,14 @@ static void xfrm_replay_timer_handler(unsigned long data)
spin_unlock(&x->lock);
}
-int xfrm_replay_check(struct xfrm_state *x, __be32 net_seq)
+int xfrm_replay_check(struct xfrm_state *x,
+ struct sk_buff *skb, __be32 net_seq)
{
u32 diff;
u32 seq = ntohl(net_seq);
if (unlikely(seq == 0))
- return -EINVAL;
+ goto err;
if (likely(seq > x->replay.seq))
return 0;
@@ -1625,14 +1633,18 @@ int xfrm_replay_check(struct xfrm_state *x, __be32 net_seq)
if (diff >= min_t(unsigned int, x->props.replay_window,
sizeof(x->replay.bitmap) * 8)) {
x->stats.replay_window++;
- return -EINVAL;
+ goto err;
}
if (x->replay.bitmap & (1U << diff)) {
x->stats.replay++;
- return -EINVAL;
+ goto err;
}
return 0;
+
+err:
+ xfrm_audit_state_replay(x, skb, net_seq);
+ return -EINVAL;
}
EXPORT_SYMBOL(xfrm_replay_check);
@@ -1995,8 +2007,8 @@ void __init xfrm_state_init(void)
}
#ifdef CONFIG_AUDITSYSCALL
-static inline void xfrm_audit_common_stateinfo(struct xfrm_state *x,
- struct audit_buffer *audit_buf)
+static inline void xfrm_audit_helper_sainfo(struct xfrm_state *x,
+ struct audit_buffer *audit_buf)
{
struct xfrm_sec_ctx *ctx = x->security;
u32 spi = ntohl(x->id.spi);
@@ -2023,6 +2035,34 @@ static inline void xfrm_audit_common_stateinfo(struct xfrm_state *x,
audit_log_format(audit_buf, " spi=%u(0x%x)", spi, spi);
}
+static inline void xfrm_audit_helper_pktinfo(struct sk_buff *skb, u16 family,
+ struct audit_buffer *audit_buf)
+{
+ struct iphdr *iph4;
+ struct ipv6hdr *iph6;
+
+ switch (family) {
+ case AF_INET:
+ iph4 = ip_hdr(skb);
+ audit_log_format(audit_buf,
+ " src=" NIPQUAD_FMT " dst=" NIPQUAD_FMT,
+ NIPQUAD(iph4->saddr),
+ NIPQUAD(iph4->daddr));
+ break;
+ case AF_INET6:
+ iph6 = ipv6_hdr(skb);
+ audit_log_format(audit_buf,
+ " src=" NIP6_FMT " dst=" NIP6_FMT
+ " flowlbl=0x%x%x%x",
+ NIP6(iph6->saddr),
+ NIP6(iph6->daddr),
+ iph6->flow_lbl[0] & 0x0f,
+ iph6->flow_lbl[1],
+ iph6->flow_lbl[2]);
+ break;
+ }
+}
+
void xfrm_audit_state_add(struct xfrm_state *x, int result,
u32 auid, u32 secid)
{
@@ -2034,7 +2074,7 @@ void xfrm_audit_state_add(struct xfrm_state *x, int result,
if (audit_buf == NULL)
return;
audit_log_format(audit_buf, " op=SAD-add res=%u", result);
- xfrm_audit_common_stateinfo(x, audit_buf);
+ xfrm_audit_helper_sainfo(x, audit_buf);
audit_log_end(audit_buf);
}
EXPORT_SYMBOL_GPL(xfrm_audit_state_add);
@@ -2050,8 +2090,107 @@ void xfrm_audit_state_delete(struct xfrm_state *x, int result,
if (audit_buf == NULL)
return;
audit_log_format(audit_buf, " op=SAD-delete res=%u", result);
- xfrm_audit_common_stateinfo(x, audit_buf);
+ xfrm_audit_helper_sainfo(x, audit_buf);
audit_log_end(audit_buf);
}
EXPORT_SYMBOL_GPL(xfrm_audit_state_delete);
+
+void xfrm_audit_state_replay_overflow(struct xfrm_state *x,
+ struct sk_buff *skb)
+{
+ struct audit_buffer *audit_buf;
+ u32 spi = ntohl(x->id.spi);
+
+ if (audit_enabled == 0)
+ return;
+ audit_buf = audit_log_start(current->audit_context, GFP_ATOMIC,
+ AUDIT_MAC_IPSEC_EVENT);
+ if (audit_buf == NULL)
+ return;
+ audit_log_format(audit_buf, " op=SA-replay-overflow");
+ xfrm_audit_helper_pktinfo(skb, x->props.family, audit_buf);
+ /* don't record the sequence number because it's inherent in this kind
+ * of audit message */
+ audit_log_format(audit_buf, " spi=%u(0x%x)", spi, spi);
+ audit_log_end(audit_buf);
+}
+EXPORT_SYMBOL_GPL(xfrm_audit_state_replay_overflow);
+
+static void xfrm_audit_state_replay(struct xfrm_state *x,
+ struct sk_buff *skb, __be32 net_seq)
+{
+ struct audit_buffer *audit_buf;
+ u32 spi = ntohl(x->id.spi);
+
+ if (audit_enabled == 0)
+ return;
+ audit_buf = audit_log_start(current->audit_context, GFP_ATOMIC,
+ AUDIT_MAC_IPSEC_EVENT);
+ if (audit_buf == NULL)
+ return;
+ audit_log_format(audit_buf, " op=SA-replayed-pkt");
+ xfrm_audit_helper_pktinfo(skb, x->props.family, audit_buf);
+ audit_log_format(audit_buf, " spi=%u(0x%x) seqno=%u",
+ spi, spi, ntohl(net_seq));
+ audit_log_end(audit_buf);
+}
+
+void xfrm_audit_state_notfound_simple(struct sk_buff *skb, u16 family)
+{
+ struct audit_buffer *audit_buf;
+
+ if (audit_enabled == 0)
+ return;
+ audit_buf = audit_log_start(current->audit_context, GFP_ATOMIC,
+ AUDIT_MAC_IPSEC_EVENT);
+ if (audit_buf == NULL)
+ return;
+ audit_log_format(audit_buf, " op=SA-notfound");
+ xfrm_audit_helper_pktinfo(skb, family, audit_buf);
+ audit_log_end(audit_buf);
+}
+EXPORT_SYMBOL_GPL(xfrm_audit_state_notfound_simple);
+
+void xfrm_audit_state_notfound(struct sk_buff *skb, u16 family,
+ __be32 net_spi, __be32 net_seq)
+{
+ struct audit_buffer *audit_buf;
+ u32 spi = ntohl(net_spi);
+
+ if (audit_enabled == 0)
+ return;
+ audit_buf = audit_log_start(current->audit_context, GFP_ATOMIC,
+ AUDIT_MAC_IPSEC_EVENT);
+ if (audit_buf == NULL)
+ return;
+ audit_log_format(audit_buf, " op=SA-notfound");
+ xfrm_audit_helper_pktinfo(skb, family, audit_buf);
+ audit_log_format(audit_buf, " spi=%u(0x%x) seqno=%u",
+ spi, spi, ntohl(net_seq));
+ audit_log_end(audit_buf);
+}
+EXPORT_SYMBOL_GPL(xfrm_audit_state_notfound);
+
+void xfrm_audit_state_icvfail(struct xfrm_state *x,
+ struct sk_buff *skb, u8 proto)
+{
+ struct audit_buffer *audit_buf;
+ __be32 net_spi;
+ __be32 net_seq;
+ u32 spi;
+
+ audit_buf = audit_log_start(current->audit_context, GFP_ATOMIC,
+ AUDIT_MAC_IPSEC_EVENT);
+ if (audit_buf == NULL)
+ return;
+ audit_log_format(audit_buf, " op=SA-icv-failure");
+ xfrm_audit_helper_pktinfo(skb, x->props.family, audit_buf);
+ if (xfrm_parse_spi(skb, proto, &net_spi, &net_seq) == 0) {
+ spi = ntohl(net_spi);
+ audit_log_format(audit_buf, " spi=%u(0x%x) seqno=%u",
+ spi, spi, ntohl(net_seq));
+ }
+ audit_log_end(audit_buf);
+}
+EXPORT_SYMBOL_GPL(xfrm_audit_state_icvfail);
#endif /* CONFIG_AUDITSYSCALL */
^ permalink raw reply related
* [PATCH] [IPV6] XFRM: Fix auditing rt6i_flags; use RTF_xxx flags instead of RTCF_xxx.
From: YOSHIFUJI Hideaki / 吉藤英明 @ 2007-12-07 18:41 UTC (permalink / raw)
To: davem; +Cc: yoshfuji, netdev
RTCF_xxx flags, defined in include/linux/in_route.h) are available for
IPv4 route (rtable) entries only. Use RTF_xxx flags instead,
defined in include/linux/ipv6_route.h, for IPv6 route entries (rt6_info).
Signed-off-by: YOSHIFUJI Hideaki <yoshfuji@linux-ipv6.org>
--
diff --git a/net/ipv6/xfrm6_policy.c b/net/ipv6/xfrm6_policy.c
index 82e27b8..b8e9eb4 100644
--- a/net/ipv6/xfrm6_policy.c
+++ b/net/ipv6/xfrm6_policy.c
@@ -233,7 +233,7 @@ __xfrm6_bundle_create(struct xfrm_policy *policy, struct xfrm_state **xfrm, int
dst_prev->output = dst_prev->xfrm->outer_mode->afinfo->output;
/* Sheit... I remember I did this right. Apparently,
* it was magically lost, so this code needs audit */
- x->u.rt6.rt6i_flags = rt0->rt6i_flags&(RTCF_BROADCAST|RTCF_MULTICAST|RTCF_LOCAL);
+ x->u.rt6.rt6i_flags = rt0->rt6i_flags&(RTF_ANYCAST|RTF_LOCAL);
x->u.rt6.rt6i_metric = rt0->rt6i_metric;
x->u.rt6.rt6i_node = rt0->rt6i_node;
x->u.rt6.rt6i_gateway = rt0->rt6i_gateway;
--
YOSHIFUJI Hideaki @ USAGI Project <yoshfuji@linux-ipv6.org>
GPG-FP : 9022 65EB 1ECF 3AD1 0BDF 80D8 4807 F894 E062 0EEA
^ permalink raw reply related
* Re: [patch 06/22] NET: DM9000: Use kthread to probe MII status when device open
From: Ben Dooks @ 2007-12-07 18:33 UTC (permalink / raw)
To: Jeff Garzik; +Cc: Ben Dooks, netdev, vince
In-Reply-To: <474780AB.20801@garzik.org>
On Fri, Nov 23, 2007 at 08:38:51PM -0500, Jeff Garzik wrote:
> seems like a delayed workqueue would be most appropriate for this.
I like the fact that the use of kthread shows the user how much
cpu time is being used by the execution of monitoring the phy. How
badly do people object to using a kthread?
--
Ben (ben@fluff.org, http://www.fluff.org/)
'a smiley only costs 4 bytes'
^ permalink raw reply
* Re: [patch 22/22] NET: DM9000: Show the MAC address source after printing MAC
From: Ben Dooks @ 2007-12-07 18:30 UTC (permalink / raw)
To: Jeff Garzik; +Cc: Ben Dooks, netdev, vince
In-Reply-To: <474781A8.2030704@garzik.org>
On Fri, Nov 23, 2007 at 08:43:04PM -0500, Jeff Garzik wrote:
> ACK patches 16-22
Is reposting here ok to get these queued for the next kernel
release, or are there people to CC: for this?
--
Ben (ben@fluff.org, http://www.fluff.org/)
'a smiley only costs 4 bytes'
^ permalink raw reply
* Re: [RFC] TCP illinois max rtt aging
From: Ilpo Järvinen @ 2007-12-07 18:27 UTC (permalink / raw)
To: David Miller; +Cc: lachlan.andrew, Netdev, quetchen
In-Reply-To: <Pine.LNX.4.64.0712071442530.18529@kivilampi-30.cs.helsinki.fi>
[-- Attachment #1: Type: TEXT/PLAIN, Size: 1911 bytes --]
On Fri, 7 Dec 2007, Ilpo Järvinen wrote:
> On Fri, 7 Dec 2007, David Miller wrote:
>
> > From: "Ilpo_Järvinen" <ilpo.jarvinen@helsinki.fi>
> > Date: Fri, 7 Dec 2007 13:05:46 +0200 (EET)
> >
> > > I guess if you get a large cumulative ACK, the amount of processing is
> > > still overwhelming (added DaveM if he has some idea how to combat it).
> > >
> > > Even a simple scenario (this isn't anything fancy at all, will occur all
> > > the time): Just one loss => rest skbs grow one by one into a single
> > > very large SACK block (and we do that efficiently for sure) => then the
> > > fast retransmit gets delivered and a cumulative ACK for whole orig_window
> > > arrives => clean_rtx_queue has to do a lot of processing. In this case we
> > > could optimize RB-tree cleanup away (by just blanking it all) but still
> > > getting rid of all those skbs is going to take a larger moment than I'd
> > > like to see.
> > >
> > > That tree blanking could be extended to cover anything which ACK more than
> > > half of the tree by just replacing the root (and dealing with potential
> > > recolorization of the root).
> >
> > Yes, it's the classic problem. But it ought to be at least
> > partially masked when TSO is in use, because we'll only process
> > a handful of SKBs. The more effectively TSO batches, the
> > less work clean_rtx_queue() will do.
>
> No, that's not what is going to happen, TSO won't help at all
> because one-by-one SACKs will fragment every single one of them
> (see tcp_match_skb_to_sack) :-(. ...So we're back in non-TSO
> case, or am I missing something?
Hmm... this could be solved though by postponing the fragmentation of a
partially sacked skb when the first sack block can (is likely) to still
grow and remove the need for fragmentation. Has some implications to
packet processing, increases burstiness a bit & tcp_max_burst kicks in too
easily.
--
i.
^ permalink raw reply
* [PATCH] Use BUILD_BUG_ON in inet_timewait_sock.c checks
From: Pavel Emelyanov @ 2007-12-07 17:46 UTC (permalink / raw)
To: David Miller; +Cc: Linux Netdev List, devel
Make the INET_TWDR_TWKILL_SLOTS vs sizeof(twdr->thread_slots)
check nicer.
Signed-off-by: Pavel Emelyanov <xemul@openvz.org>
---
diff --git a/net/ipv4/inet_timewait_sock.c b/net/ipv4/inet_timewait_sock.c
index a60b99e..d43e787 100644
--- a/net/ipv4/inet_timewait_sock.c
+++ b/net/ipv4/inet_timewait_sock.c
@@ -194,16 +194,14 @@ out:
EXPORT_SYMBOL_GPL(inet_twdr_hangman);
-extern void twkill_slots_invalid(void);
-
void inet_twdr_twkill_work(struct work_struct *work)
{
struct inet_timewait_death_row *twdr =
container_of(work, struct inet_timewait_death_row, twkill_work);
int i;
- if ((INET_TWDR_TWKILL_SLOTS - 1) > (sizeof(twdr->thread_slots) * 8))
- twkill_slots_invalid();
+ BUILD_BUG_ON((INET_TWDR_TWKILL_SLOTS - 1) >
+ (sizeof(twdr->thread_slots) * 8));
while (twdr->thread_slots) {
spin_lock_bh(&twdr->death_lock);
^ permalink raw reply related
* [PATCH] Use BUILD_BUG_ON for tcp_skb_cb size checking
From: Pavel Emelyanov @ 2007-12-07 17:42 UTC (permalink / raw)
To: David Miller; +Cc: Linux Netdev List, devel
The sizeof(struct tcp_skb_cb) should not be less than the
sizeof(skb->cb). This is checked in net/ipv4/tcp.c, but
this check can be made more gracefully.
Signed-off-by: Pavel Emelyanov <xemul@openvz.org>
---
diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
index 8e65182..c8bebd3 100644
--- a/net/ipv4/tcp.c
+++ b/net/ipv4/tcp.c
@@ -2411,7 +2411,6 @@ void tcp_done(struct sock *sk)
}
EXPORT_SYMBOL_GPL(tcp_done);
-extern void __skb_cb_too_small_for_tcp(int, int);
extern struct tcp_congestion_ops tcp_reno;
static __initdata unsigned long thash_entries;
@@ -2430,9 +2429,7 @@ void __init tcp_init(void)
unsigned long limit;
int order, i, max_share;
- if (sizeof(struct tcp_skb_cb) > sizeof(skb->cb))
- __skb_cb_too_small_for_tcp(sizeof(struct tcp_skb_cb),
- sizeof(skb->cb));
+ BUILD_BUG_ON(sizeof(struct tcp_skb_cb) > sizeof(skb->cb));
tcp_hashinfo.bind_bucket_cachep =
kmem_cache_create("tcp_bind_bucket",
^ permalink raw reply related
* [PATCH] XFRM: assorted IPsec fixups
From: Paul Moore @ 2007-12-07 17:11 UTC (permalink / raw)
To: netdev, linux-audit, selinux
This patch fixes a number of small but potentially troublesome things in the
XFRM/IPsec code:
* Use the 'audit_enabled' variable already in include/linux/audit.h
Removed the need for extern declarations local to each XFRM audit fuction
* Convert 'sid' to 'secid'
The 'sid' name is specific to SELinux, 'secid' is the common naming
convention used by the kernel when refering to tokenized LSM labels
* Convert address display to use standard NIP* macros
Similar to what was recently done with the SPD audit code, this also
includes the removal of some unnecessary memcpy() calls
* Move common code to xfrm_audit_common_stateinfo()
Code consolidation from the "less is more" book on software development
* Convert the SPI in audit records to host byte order
The current SPI values in the audit record are being displayed in
network byte order, probably not what was intended
* Proper spacing around commas in function arguments
Minor style tweak since I was already touching the code
Signed-off-by: Paul Moore <paul.moore@hp.com>
---
include/linux/xfrm.h | 2 +
include/net/xfrm.h | 18 ++++++------
net/xfrm/xfrm_policy.c | 15 +++++-----
net/xfrm/xfrm_state.c | 69 +++++++++++++++++++++--------------------------
security/selinux/xfrm.c | 20 +++++++-------
5 files changed, 58 insertions(+), 66 deletions(-)
diff --git a/include/linux/xfrm.h b/include/linux/xfrm.h
index b58adc5..f75a337 100644
--- a/include/linux/xfrm.h
+++ b/include/linux/xfrm.h
@@ -31,7 +31,7 @@ struct xfrm_sec_ctx {
__u8 ctx_doi;
__u8 ctx_alg;
__u16 ctx_len;
- __u32 ctx_sid;
+ __u32 ctx_secid;
char ctx_str[0];
};
diff --git a/include/net/xfrm.h b/include/net/xfrm.h
index 58dfa82..c02e230 100644
--- a/include/net/xfrm.h
+++ b/include/net/xfrm.h
@@ -462,7 +462,7 @@ struct xfrm_audit
};
#ifdef CONFIG_AUDITSYSCALL
-static inline struct audit_buffer *xfrm_audit_start(u32 auid, u32 sid)
+static inline struct audit_buffer *xfrm_audit_start(u32 auid, u32 secid)
{
struct audit_buffer *audit_buf = NULL;
char *secctx;
@@ -475,8 +475,8 @@ static inline struct audit_buffer *xfrm_audit_start(u32 auid, u32 sid)
audit_log_format(audit_buf, "auid=%u", auid);
- if (sid != 0 &&
- security_secid_to_secctx(sid, &secctx, &secctx_len) == 0) {
+ if (secid != 0 &&
+ security_secid_to_secctx(secid, &secctx, &secctx_len) == 0) {
audit_log_format(audit_buf, " subj=%s", secctx);
security_release_secctx(secctx, secctx_len);
} else
@@ -485,13 +485,13 @@ static inline struct audit_buffer *xfrm_audit_start(u32 auid, u32 sid)
}
extern void xfrm_audit_policy_add(struct xfrm_policy *xp, int result,
- u32 auid, u32 sid);
+ u32 auid, u32 secid);
extern void xfrm_audit_policy_delete(struct xfrm_policy *xp, int result,
- u32 auid, u32 sid);
+ u32 auid, u32 secid);
extern void xfrm_audit_state_add(struct xfrm_state *x, int result,
- u32 auid, u32 sid);
+ u32 auid, u32 secid);
extern void xfrm_audit_state_delete(struct xfrm_state *x, int result,
- u32 auid, u32 sid);
+ u32 auid, u32 secid);
#else
#define xfrm_audit_policy_add(x, r, a, s) do { ; } while (0)
#define xfrm_audit_policy_delete(x, r, a, s) do { ; } while (0)
@@ -621,13 +621,13 @@ extern int xfrm_selector_match(struct xfrm_selector *sel, struct flowi *fl,
#ifdef CONFIG_SECURITY_NETWORK_XFRM
/* If neither has a context --> match
- * Otherwise, both must have a context and the sids, doi, alg must match
+ * Otherwise, both must have a context and the secids, doi, alg must match
*/
static inline int xfrm_sec_ctx_match(struct xfrm_sec_ctx *s1, struct xfrm_sec_ctx *s2)
{
return ((!s1 && !s2) ||
(s1 && s2 &&
- (s1->ctx_sid == s2->ctx_sid) &&
+ (s1->ctx_secid == s2->ctx_secid) &&
(s1->ctx_doi == s2->ctx_doi) &&
(s1->ctx_alg == s2->ctx_alg)));
}
diff --git a/net/xfrm/xfrm_policy.c b/net/xfrm/xfrm_policy.c
index b702bd8..75f25c4 100644
--- a/net/xfrm/xfrm_policy.c
+++ b/net/xfrm/xfrm_policy.c
@@ -23,6 +23,7 @@
#include <linux/netfilter.h>
#include <linux/module.h>
#include <linux/cache.h>
+#include <linux/audit.h>
#include <net/xfrm.h>
#include <net/ip.h>
@@ -2150,15 +2151,14 @@ static inline void xfrm_audit_common_policyinfo(struct xfrm_policy *xp,
}
}
-void
-xfrm_audit_policy_add(struct xfrm_policy *xp, int result, u32 auid, u32 sid)
+void xfrm_audit_policy_add(struct xfrm_policy *xp, int result,
+ u32 auid, u32 secid)
{
struct audit_buffer *audit_buf;
- extern int audit_enabled;
if (audit_enabled == 0)
return;
- audit_buf = xfrm_audit_start(sid, auid);
+ audit_buf = xfrm_audit_start(secid, auid);
if (audit_buf == NULL)
return;
audit_log_format(audit_buf, " op=SPD-add res=%u", result);
@@ -2167,15 +2167,14 @@ xfrm_audit_policy_add(struct xfrm_policy *xp, int result, u32 auid, u32 sid)
}
EXPORT_SYMBOL_GPL(xfrm_audit_policy_add);
-void
-xfrm_audit_policy_delete(struct xfrm_policy *xp, int result, u32 auid, u32 sid)
+void xfrm_audit_policy_delete(struct xfrm_policy *xp, int result,
+ u32 auid, u32 secid)
{
struct audit_buffer *audit_buf;
- extern int audit_enabled;
if (audit_enabled == 0)
return;
- audit_buf = xfrm_audit_start(sid, auid);
+ audit_buf = xfrm_audit_start(secid, auid);
if (audit_buf == NULL)
return;
audit_log_format(audit_buf, " op=SPD-delete res=%u", result);
diff --git a/net/xfrm/xfrm_state.c b/net/xfrm/xfrm_state.c
index cf43c49..b291a82 100644
--- a/net/xfrm/xfrm_state.c
+++ b/net/xfrm/xfrm_state.c
@@ -19,6 +19,7 @@
#include <linux/ipsec.h>
#include <linux/module.h>
#include <linux/cache.h>
+#include <linux/audit.h>
#include <asm/uaccess.h>
#include "xfrm_hash.h"
@@ -1997,67 +1998,59 @@ void __init xfrm_state_init(void)
static inline void xfrm_audit_common_stateinfo(struct xfrm_state *x,
struct audit_buffer *audit_buf)
{
- if (x->security)
- audit_log_format(audit_buf, " sec_alg=%u sec_doi=%u sec_obj=%s",
- x->security->ctx_alg, x->security->ctx_doi,
- x->security->ctx_str);
+ struct xfrm_sec_ctx *ctx = x->security;
+ u32 spi = ntohl(x->id.spi);
- switch(x->props.family) {
- case AF_INET:
- audit_log_format(audit_buf, " src=%u.%u.%u.%u dst=%u.%u.%u.%u",
- NIPQUAD(x->props.saddr.a4),
- NIPQUAD(x->id.daddr.a4));
- break;
- case AF_INET6:
- {
- struct in6_addr saddr6, daddr6;
-
- memcpy(&saddr6, x->props.saddr.a6,
- sizeof(struct in6_addr));
- memcpy(&daddr6, x->id.daddr.a6,
- sizeof(struct in6_addr));
- audit_log_format(audit_buf,
- " src=" NIP6_FMT " dst=" NIP6_FMT,
- NIP6(saddr6), NIP6(daddr6));
- }
- break;
- }
+ if (ctx)
+ audit_log_format(audit_buf, " sec_alg=%u sec_doi=%u sec_obj=%s",
+ ctx->ctx_alg, ctx->ctx_doi, ctx->ctx_str);
+
+ switch(x->props.family) {
+ case AF_INET:
+ audit_log_format(audit_buf,
+ " src=" NIPQUAD_FMT " dst=" NIPQUAD_FMT,
+ NIPQUAD(x->props.saddr.a4),
+ NIPQUAD(x->id.daddr.a4));
+ break;
+ case AF_INET6:
+ audit_log_format(audit_buf,
+ " src=" NIP6_FMT " dst=" NIP6_FMT,
+ NIP6(*(struct in6_addr *)x->props.saddr.a6),
+ NIP6(*(struct in6_addr *)x->id.daddr.a6));
+ break;
+ }
+
+ audit_log_format(audit_buf, " spi=%u(0x%x)", spi, spi);
}
-void
-xfrm_audit_state_add(struct xfrm_state *x, int result, u32 auid, u32 sid)
+void xfrm_audit_state_add(struct xfrm_state *x, int result,
+ u32 auid, u32 secid)
{
struct audit_buffer *audit_buf;
- extern int audit_enabled;
if (audit_enabled == 0)
return;
- audit_buf = xfrm_audit_start(sid, auid);
+ audit_buf = xfrm_audit_start(secid, auid);
if (audit_buf == NULL)
return;
- audit_log_format(audit_buf, " op=SAD-add res=%u",result);
+ audit_log_format(audit_buf, " op=SAD-add res=%u", result);
xfrm_audit_common_stateinfo(x, audit_buf);
- audit_log_format(audit_buf, " spi=%lu(0x%lx)",
- (unsigned long)x->id.spi, (unsigned long)x->id.spi);
audit_log_end(audit_buf);
}
EXPORT_SYMBOL_GPL(xfrm_audit_state_add);
-void
-xfrm_audit_state_delete(struct xfrm_state *x, int result, u32 auid, u32 sid)
+void xfrm_audit_state_delete(struct xfrm_state *x, int result,
+ u32 auid, u32 secid)
{
struct audit_buffer *audit_buf;
- extern int audit_enabled;
if (audit_enabled == 0)
return;
- audit_buf = xfrm_audit_start(sid, auid);
+ audit_buf = xfrm_audit_start(secid, auid);
if (audit_buf == NULL)
return;
- audit_log_format(audit_buf, " op=SAD-delete res=%u",result);
+ audit_log_format(audit_buf, " op=SAD-delete res=%u", result);
xfrm_audit_common_stateinfo(x, audit_buf);
- audit_log_format(audit_buf, " spi=%lu(0x%lx)",
- (unsigned long)x->id.spi, (unsigned long)x->id.spi);
audit_log_end(audit_buf);
}
EXPORT_SYMBOL_GPL(xfrm_audit_state_delete);
diff --git a/security/selinux/xfrm.c b/security/selinux/xfrm.c
index e076039..c925880 100644
--- a/security/selinux/xfrm.c
+++ b/security/selinux/xfrm.c
@@ -85,7 +85,7 @@ int selinux_xfrm_policy_lookup(struct xfrm_policy *xp, u32 fl_secid, u8 dir)
if (!selinux_authorizable_ctx(ctx))
return -EINVAL;
- sel_sid = ctx->ctx_sid;
+ sel_sid = ctx->ctx_secid;
}
else
/*
@@ -132,7 +132,7 @@ int selinux_xfrm_state_pol_flow_match(struct xfrm_state *x, struct xfrm_policy *
/* Not a SELinux-labeled SA */
return 0;
- state_sid = x->security->ctx_sid;
+ state_sid = x->security->ctx_secid;
if (fl->secid != state_sid)
return 0;
@@ -175,13 +175,13 @@ int selinux_xfrm_decode_session(struct sk_buff *skb, u32 *sid, int ckall)
struct xfrm_sec_ctx *ctx = x->security;
if (!sid_set) {
- *sid = ctx->ctx_sid;
+ *sid = ctx->ctx_secid;
sid_set = 1;
if (!ckall)
break;
}
- else if (*sid != ctx->ctx_sid)
+ else if (*sid != ctx->ctx_secid)
return -EINVAL;
}
}
@@ -232,7 +232,7 @@ static int selinux_xfrm_sec_ctx_alloc(struct xfrm_sec_ctx **ctxp,
ctx->ctx_str[str_len] = 0;
rc = security_context_to_sid(ctx->ctx_str,
str_len,
- &ctx->ctx_sid);
+ &ctx->ctx_secid);
if (rc)
goto out;
@@ -240,7 +240,7 @@ static int selinux_xfrm_sec_ctx_alloc(struct xfrm_sec_ctx **ctxp,
/*
* Does the subject have permission to set security context?
*/
- rc = avc_has_perm(tsec->sid, ctx->ctx_sid,
+ rc = avc_has_perm(tsec->sid, ctx->ctx_secid,
SECCLASS_ASSOCIATION,
ASSOCIATION__SETCONTEXT, NULL);
if (rc)
@@ -264,7 +264,7 @@ not_from_user:
ctx->ctx_doi = XFRM_SC_DOI_LSM;
ctx->ctx_alg = XFRM_SC_ALG_SELINUX;
- ctx->ctx_sid = sid;
+ ctx->ctx_secid = sid;
ctx->ctx_len = str_len;
memcpy(ctx->ctx_str,
ctx_str,
@@ -341,7 +341,7 @@ int selinux_xfrm_policy_delete(struct xfrm_policy *xp)
int rc = 0;
if (ctx)
- rc = avc_has_perm(tsec->sid, ctx->ctx_sid,
+ rc = avc_has_perm(tsec->sid, ctx->ctx_secid,
SECCLASS_ASSOCIATION,
ASSOCIATION__SETCONTEXT, NULL);
@@ -383,7 +383,7 @@ int selinux_xfrm_state_delete(struct xfrm_state *x)
int rc = 0;
if (ctx)
- rc = avc_has_perm(tsec->sid, ctx->ctx_sid,
+ rc = avc_has_perm(tsec->sid, ctx->ctx_secid,
SECCLASS_ASSOCIATION,
ASSOCIATION__SETCONTEXT, NULL);
@@ -412,7 +412,7 @@ int selinux_xfrm_sock_rcv_skb(u32 isec_sid, struct sk_buff *skb,
if (x && selinux_authorizable_xfrm(x)) {
struct xfrm_sec_ctx *ctx = x->security;
- sel_sid = ctx->ctx_sid;
+ sel_sid = ctx->ctx_secid;
break;
}
}
^ permalink raw reply related
* Re: TCP event tracking via netlink...
From: Ilpo Järvinen @ 2007-12-07 16:43 UTC (permalink / raw)
To: David Miller; +Cc: Netdev
In-Reply-To: <20071206.023346.256200942.davem@davemloft.net>
[-- Attachment #1: Type: TEXT/PLAIN, Size: 6292 bytes --]
On Thu, 6 Dec 2007, David Miller wrote:
> From: "Ilpo_Järvinen" <ilpo.jarvinen@helsinki.fi>
> Date: Thu, 6 Dec 2007 01:18:28 +0200 (EET)
>
> > On Wed, 5 Dec 2007, David Miller wrote:
> >
> > > I assume you're using something like carefully crafted printk's,
> > > kprobes, or even ad-hoc statistic counters. That's what I used to do
> > > :-)
> >
> > No, that's not at all what I do :-). I usually look time-seq graphs
> > expect for the cases when I just find things out by reading code (or
> > by just thinking of it).
>
> Can you briefly detail what graph tools and command lines
> you are using?
I have a tool called Sealion but it's behind NDA (making it open source
has been talked for long but I don't have idea why it hasn't realized
yet). It's mostly tcl/tk code is, by no means nice or clean desing nor
quality (I'll leave details why I think it's that way out of this
discussion :-)). Produces svgs. Usually I'm have the things I need in
the standard sent+ACK+SACKs(+win) graph it produces. The result is quite
similar to what tcptrace+xplot produces but xplot UI is really horrible,
IMHO.
If I have to deal with tcpdump output only, it takes considerable amount
of time to do computations with bc to come up with the same understanding
by just reading tcpdumps.
> The last time I did graphing to analyze things, the tools
> were hit-or-miss.
Yeah, this is definately true. Open source graphing tools I know are
really not that astonishing :-(. I've tried to look for better tools
as well but with little success.
> > Much of the info is available in tcpdump already, it's just hard to read
> > without graphing it first because there are some many overlapping things
> > to track in two-dimensional space.
> >
> > ...But yes, I have to admit that couple of problems come to my mind
> > where having some variable from tcp_sock would have made the problem
> > more obvious.
>
> The most important are the cwnd and ssthresh, which you could guess
> using graphs but it is important to know on a packet to packet
> basis why we might have sent a packet or not because this has
> rippling effects down the rest of the RTT.
Couple of points:
In order to evaluate validity of some action, one might need more than
one packet from the history.
Answer to the why we have sent a packet is rather simple (excluding RTOs):
cwnd > packets_in_flight and data was available. No, it's not at all
complicated. Though I might be too biased toward non-application limited
cases which make the formula even simpler because everything is basically
ACK clocked.
To really tell what caused changes between cwnd and/or packets_in_flight
one usually needs some history or more fine-grained approach, once per
packet is way too wide gap. It tells just what happened, not why, unless
you're really familiar with the state machine and can make the right
guess.
> > Not sure what is the benefit of having distributions with it because
> > those people hardly report problems anyway to here, they're just too
> > happy with TCP performance unless we print something to their logs,
> > which implies that we must setup a *_ON() condition :-(.
>
> That may be true, but if we could integrate the information with
> tcpdumps, we could gather internal state using tools the user
> already has available.
It would definately help if we could, but that of course depends on
getting the reports in the first place.
> Imagine if tcpdump printed out:
>
> 02:26:14.865805 IP $SRC > $DEST: . 11226:12686(1460) ack 0 win 108
> ss_thresh: 129 cwnd: 133 packets_out: 132
>
> or something like that.
How about this:
02:26:14.865805 IP $SRC > $DEST: . ack 11226 win 108 <...sack 1 {15606:18526}
17066:18526 0->S sacktag_one l0 s1 r0 f4 pc1 ...
11226:12686 ---- clean_rtx_queue ...
11226:12686 0->L mark_head_lost l1 s1 r0 f4 pc1 ...
12686:14146 0->L mark_head_lost l2 s1 r0 f4 pc1 ...
11226:12686 L->LRe retransmit_skb l2 s1 r1 f4 pc1 ...
...would make the bug in sack processing relatively obvious (yes, it
has an intentional flaw in it, points from find it :-))... That would
be something I'd like to have right now.
> But sometimes the algorithms are working as designed, it's just that
> they provide poor pipe utilization and CWND analysis embedded inside
> of a tcpdump would be one way to see that as well as determine the
> flaw in the algorithm.
Fair enough.
> It is untested since I didn't write the userland app yet to see that
> proper things get logged. Basically you could run a daemon that
> writes per-connection traces into files based upon the incoming
> netlink events. Later, using the binary pcap file and these traces,
> you can piece together traces like the above using the timestamps
> etc. to match up pcap packets to ones from the TCP logger.
>
> The userland tools could do analysis and print pre-cooked state diff
> logs, like "this ACK raised CWND by one" or whatever else you wanted
> to know.
Obviously a collection of useful userland tools seems here at least as
important as the existance of the interface.
> It's nice that an expert like you can look at graphs and understand,
> but we'd like to create more experts and besides reading code one
> way to become an expert is to be able to extrace live real data
> from the kernel's working state and try to understand how things
> got that way. This information is permanently lost currently.
IMHO this problem is in such caliber that no human can track efficiently
more than a couple of packets of a TCP flow from text only view, without
headaches I mean, or are you able to do that at ease? And we're talking
here about the people who have just begun to deal with TCP. ...For me
especially those nearly identical seqnos all around are too overwhelming
to track in any sane way and I'd expect that most feel the same way.
I'd state my point different way around (with the terms you chose): it is
very difficult to become an expert without looking some graphs, we may
disagree and that's fine :-). I think it's because one would then have no
idea about larger picture (and about the very _relevant_ past/future) when
looking just a single line at a time from the tcpdump (or equivalent). Sad
thing is that a good tool to do the visulization might not exist.
--
i.
^ permalink raw reply
* [PATCH net-2.6.25] Cleanup sysctl manipulations in devinet.c
From: Pavel Emelyanov @ 2007-12-07 16:25 UTC (permalink / raw)
To: David Miller; +Cc: Linux Netdev List, devel
This includes:
* moving neigh_sysctl_(un)register calls inside
devinet_sysctl_(un)register ones, as they are always
called in pairs;
* making __devinet_sysctl_unregister() to unregister
the ipv4_devconf struct, while original devinet_sysctl_unregister()
works with the in_device to handle both - devconf and
neigh sysctls;
* make stubs for CONFIG_SYSCTL=n case to get rid of
in-code ifdefs.
Signed-off-by: Pavel Emelyanov <xemul@openvz.org>
---
diff --git a/net/ipv4/devinet.c b/net/ipv4/devinet.c
index 0b5f042..872883e 100644
--- a/net/ipv4/devinet.c
+++ b/net/ipv4/devinet.c
@@ -99,7 +99,14 @@ static void inet_del_ifa(struct in_device *in_dev, struct in_ifaddr **ifap,
int destroy);
#ifdef CONFIG_SYSCTL
static void devinet_sysctl_register(struct in_device *idev);
-static void devinet_sysctl_unregister(struct ipv4_devconf *p);
+static void devinet_sysctl_unregister(struct in_device *idev);
+#else
+static inline void devinet_sysctl_register(struct in_device *idev)
+{
+}
+static inline void devinet_sysctl_unregister(struct in_device *idev)
+{
+}
#endif
/* Locks all the inet devices. */
@@ -163,17 +170,10 @@ static struct in_device *inetdev_init(struct net_device *dev)
goto out_kfree;
/* Reference in_dev->dev */
dev_hold(dev);
-#ifdef CONFIG_SYSCTL
- neigh_sysctl_register(dev, in_dev->arp_parms, NET_IPV4,
- NET_IPV4_NEIGH, "ipv4", NULL, NULL);
-#endif
-
/* Account for reference dev->ip_ptr (below) */
in_dev_hold(in_dev);
-#ifdef CONFIG_SYSCTL
devinet_sysctl_register(in_dev);
-#endif
ip_mc_init_dev(in_dev);
if (dev->flags & IFF_UP)
ip_mc_up(in_dev);
@@ -212,15 +212,9 @@ static void inetdev_destroy(struct in_device *in_dev)
inet_free_ifa(ifa);
}
-#ifdef CONFIG_SYSCTL
- devinet_sysctl_unregister(&in_dev->cnf);
-#endif
-
dev->ip_ptr = NULL;
-#ifdef CONFIG_SYSCTL
- neigh_sysctl_unregister(in_dev->arp_parms);
-#endif
+ devinet_sysctl_unregister(in_dev);
neigh_parms_release(&arp_tbl, in_dev->arp_parms);
arp_ifdown(dev);
@@ -1114,13 +1108,8 @@ static int inetdev_event(struct notifier_block *this, unsigned long event,
*/
inetdev_changename(dev, in_dev);
-#ifdef CONFIG_SYSCTL
- devinet_sysctl_unregister(&in_dev->cnf);
- neigh_sysctl_unregister(in_dev->arp_parms);
- neigh_sysctl_register(dev, in_dev->arp_parms, NET_IPV4,
- NET_IPV4_NEIGH, "ipv4", NULL, NULL);
+ devinet_sysctl_unregister(in_dev);
devinet_sysctl_register(in_dev);
-#endif
break;
}
out:
@@ -1519,21 +1508,31 @@ out:
return;
}
+static void __devinet_sysctl_unregister(struct ipv4_devconf *cnf)
+{
+ struct devinet_sysctl_table *t = cnf->sysctl;
+
+ if (t == NULL)
+ return;
+
+ cnf->sysctl = NULL;
+ unregister_sysctl_table(t->sysctl_header);
+ kfree(t->dev_name);
+ kfree(t);
+}
+
static void devinet_sysctl_register(struct in_device *idev)
{
- return __devinet_sysctl_register(idev->dev->name, idev->dev->ifindex,
+ neigh_sysctl_register(idev->dev, idev->arp_parms, NET_IPV4,
+ NET_IPV4_NEIGH, "ipv4", NULL, NULL);
+ __devinet_sysctl_register(idev->dev->name, idev->dev->ifindex,
&idev->cnf);
}
-static void devinet_sysctl_unregister(struct ipv4_devconf *p)
+static void devinet_sysctl_unregister(struct in_device *idev)
{
- if (p->sysctl) {
- struct devinet_sysctl_table *t = p->sysctl;
- p->sysctl = NULL;
- unregister_sysctl_table(t->sysctl_header);
- kfree(t->dev_name);
- kfree(t);
- }
+ __devinet_sysctl_unregister(&idev->cnf);
+ neigh_sysctl_unregister(idev->arp_parms);
}
#endif
--
1.5.3.4
^ permalink raw reply related
* [PATCH net-2.6.25] Cleanup IN_DEV_MFORWARD macro
From: Pavel Emelyanov @ 2007-12-07 16:19 UTC (permalink / raw)
To: David Miller; +Cc: Linux Netdev List, devel, Herbert Xu
This is essentially IN_DEV_ANDCONF with proper arguments.
Signed-off-by: Pavel Emelyanov <xemul@openvz.org>
---
diff --git a/include/linux/inetdevice.h b/include/linux/inetdevice.h
index dd093ea..962a062 100644
--- a/include/linux/inetdevice.h
+++ b/include/linux/inetdevice.h
@@ -78,9 +78,7 @@ static inline void ipv4_devconf_setall(struct in_device *in_dev)
(max(IPV4_DEVCONF_ALL(attr), IN_DEV_CONF_GET((in_dev), attr)))
#define IN_DEV_FORWARD(in_dev) IN_DEV_CONF_GET((in_dev), FORWARDING)
-#define IN_DEV_MFORWARD(in_dev) (IPV4_DEVCONF_ALL(MC_FORWARDING) && \
- IPV4_DEVCONF((in_dev)->cnf, \
- MC_FORWARDING))
+#define IN_DEV_MFORWARD(in_dev) IN_DEV_ANDCONF((in_dev), MC_FORWARDING)
#define IN_DEV_RPFILTER(in_dev) IN_DEV_ANDCONF((in_dev), RP_FILTER)
#define IN_DEV_SOURCE_ROUTE(in_dev) IN_DEV_ANDCONF((in_dev), \
ACCEPT_SOURCE_ROUTE)
--
1.5.3.4
^ permalink raw reply related
* [PATCH 2.6.25 3/3] ipv4: last default route is a fib table property
From: Denis V. Lunev @ 2007-12-07 16:11 UTC (permalink / raw)
To: davem; +Cc: containers, devel, netdev
ipv4: last default route is a fib table property
Signed-off-by: Denis V. Lunev <den@openvz.org>
Acked-by: Alexey Kuznetsov <kuznet@ms2.inr.ac.ru>
---
include/net/ip_fib.h | 1 +
net/ipv4/fib_hash.c | 16 ++++++++--------
net/ipv4/fib_trie.c | 18 +++++++++---------
3 files changed, 18 insertions(+), 17 deletions(-)
diff --git a/include/net/ip_fib.h b/include/net/ip_fib.h
index 690fb4d..d70b9b4 100644
--- a/include/net/ip_fib.h
+++ b/include/net/ip_fib.h
@@ -141,6 +141,7 @@ struct fib_table {
struct hlist_node tb_hlist;
u32 tb_id;
unsigned tb_stamp;
+ int tb_default;
int (*tb_lookup)(struct fib_table *tb, const struct flowi *flp, struct fib_result *res);
int (*tb_insert)(struct fib_table *, struct fib_config *);
int (*tb_delete)(struct fib_table *, struct fib_config *);
diff --git a/net/ipv4/fib_hash.c b/net/ipv4/fib_hash.c
index a52b570..481de47 100644
--- a/net/ipv4/fib_hash.c
+++ b/net/ipv4/fib_hash.c
@@ -272,8 +272,6 @@ out:
return err;
}
-static int fn_hash_last_dflt=-1;
-
static void
fn_hash_select_default(struct fib_table *tb, const struct flowi *flp, struct fib_result *res)
{
@@ -314,9 +312,9 @@ fn_hash_select_default(struct fib_table *tb, const struct flowi *flp, struct fib
if (next_fi != res->fi)
break;
} else if (!fib_detect_death(fi, order, &last_resort,
- &last_idx, fn_hash_last_dflt)) {
+ &last_idx, tb->tb_default)) {
fib_result_assign(res, fi);
- fn_hash_last_dflt = order;
+ tb->tb_default = order;
goto out;
}
fi = next_fi;
@@ -325,19 +323,20 @@ fn_hash_select_default(struct fib_table *tb, const struct flowi *flp, struct fib
}
if (order <= 0 || fi == NULL) {
- fn_hash_last_dflt = -1;
+ tb->tb_default = -1;
goto out;
}
- if (!fib_detect_death(fi, order, &last_resort, &last_idx, fn_hash_last_dflt)) {
+ if (!fib_detect_death(fi, order, &last_resort, &last_idx,
+ tb->tb_default)) {
fib_result_assign(res, fi);
- fn_hash_last_dflt = order;
+ tb->tb_default = order;
goto out;
}
if (last_idx >= 0)
fib_result_assign(res, last_resort);
- fn_hash_last_dflt = last_idx;
+ tb->tb_default = last_idx;
out:
read_unlock(&fib_hash_lock);
}
@@ -772,6 +771,7 @@ struct fib_table * __init fib_hash_init(u32 id)
return NULL;
tb->tb_id = id;
+ tb->tb_default = -1;
tb->tb_lookup = fn_hash_lookup;
tb->tb_insert = fn_hash_insert;
tb->tb_delete = fn_hash_delete;
diff --git a/net/ipv4/fib_trie.c b/net/ipv4/fib_trie.c
index 29a06af..850165a 100644
--- a/net/ipv4/fib_trie.c
+++ b/net/ipv4/fib_trie.c
@@ -1779,8 +1779,6 @@ static int fn_trie_flush(struct fib_table *tb)
return found;
}
-static int trie_last_dflt = -1;
-
static void
fn_trie_select_default(struct fib_table *tb, const struct flowi *flp, struct fib_result *res)
{
@@ -1827,28 +1825,29 @@ fn_trie_select_default(struct fib_table *tb, const struct flowi *flp, struct fib
if (next_fi != res->fi)
break;
} else if (!fib_detect_death(fi, order, &last_resort,
- &last_idx, trie_last_dflt)) {
+ &last_idx, tb->tb_default)) {
fib_result_assign(res, fi);
- trie_last_dflt = order;
+ tb->tb_default = order;
goto out;
}
fi = next_fi;
order++;
}
if (order <= 0 || fi == NULL) {
- trie_last_dflt = -1;
+ tb->tb_default = -1;
goto out;
}
- if (!fib_detect_death(fi, order, &last_resort, &last_idx, trie_last_dflt)) {
+ if (!fib_detect_death(fi, order, &last_resort, &last_idx,
+ tb->tb_default)) {
fib_result_assign(res, fi);
- trie_last_dflt = order;
+ tb->tb_default = order;
goto out;
}
if (last_idx >= 0)
fib_result_assign(res, last_resort);
- trie_last_dflt = last_idx;
- out:;
+ tb->tb_default = last_idx;
+out:
rcu_read_unlock();
}
@@ -1975,6 +1974,7 @@ struct fib_table * __init fib_hash_init(u32 id)
return NULL;
tb->tb_id = id;
+ tb->tb_default = -1;
tb->tb_lookup = fn_trie_lookup;
tb->tb_insert = fn_trie_insert;
tb->tb_delete = fn_trie_delete;
--
1.5.3.rc5
^ permalink raw reply related
* [PATCH 2.6.25 2/3] ipv4: unify assignment of fi to fib_result
From: Denis V. Lunev @ 2007-12-07 16:10 UTC (permalink / raw)
To: davem; +Cc: containers, devel, netdev
ipv4: unify assignment of fi to fib_result
Signed-off-by: Denis V. Lunev <den@openvz.org>
Acked-by: Alexey Kuznetsov <kuznet@ms2.inr.ac.ru>
---
net/ipv4/fib_hash.c | 19 ++++---------------
net/ipv4/fib_lookup.h | 10 ++++++++++
net/ipv4/fib_trie.c | 19 ++++---------------
3 files changed, 18 insertions(+), 30 deletions(-)
diff --git a/net/ipv4/fib_hash.c b/net/ipv4/fib_hash.c
index 76bb7fd..a52b570 100644
--- a/net/ipv4/fib_hash.c
+++ b/net/ipv4/fib_hash.c
@@ -315,10 +315,7 @@ fn_hash_select_default(struct fib_table *tb, const struct flowi *flp, struct fib
break;
} else if (!fib_detect_death(fi, order, &last_resort,
&last_idx, fn_hash_last_dflt)) {
- if (res->fi)
- fib_info_put(res->fi);
- res->fi = fi;
- atomic_inc(&fi->fib_clntref);
+ fib_result_assign(res, fi);
fn_hash_last_dflt = order;
goto out;
}
@@ -333,21 +330,13 @@ fn_hash_select_default(struct fib_table *tb, const struct flowi *flp, struct fib
}
if (!fib_detect_death(fi, order, &last_resort, &last_idx, fn_hash_last_dflt)) {
- if (res->fi)
- fib_info_put(res->fi);
- res->fi = fi;
- atomic_inc(&fi->fib_clntref);
+ fib_result_assign(res, fi);
fn_hash_last_dflt = order;
goto out;
}
- if (last_idx >= 0) {
- if (res->fi)
- fib_info_put(res->fi);
- res->fi = last_resort;
- if (last_resort)
- atomic_inc(&last_resort->fib_clntref);
- }
+ if (last_idx >= 0)
+ fib_result_assign(res, last_resort);
fn_hash_last_dflt = last_idx;
out:
read_unlock(&fib_hash_lock);
diff --git a/net/ipv4/fib_lookup.h b/net/ipv4/fib_lookup.h
index 6c9dd42..26ee66d 100644
--- a/net/ipv4/fib_lookup.h
+++ b/net/ipv4/fib_lookup.h
@@ -38,4 +38,14 @@ extern int fib_detect_death(struct fib_info *fi, int order,
struct fib_info **last_resort,
int *last_idx, int dflt);
+static inline void fib_result_assign(struct fib_result *res,
+ struct fib_info *fi)
+{
+ if (res->fi != NULL)
+ fib_info_put(res->fi);
+ res->fi = fi;
+ if (fi != NULL)
+ atomic_inc(&fi->fib_clntref);
+}
+
#endif /* _FIB_LOOKUP_H */
diff --git a/net/ipv4/fib_trie.c b/net/ipv4/fib_trie.c
index 914a0d2..29a06af 100644
--- a/net/ipv4/fib_trie.c
+++ b/net/ipv4/fib_trie.c
@@ -1828,10 +1828,7 @@ fn_trie_select_default(struct fib_table *tb, const struct flowi *flp, struct fib
break;
} else if (!fib_detect_death(fi, order, &last_resort,
&last_idx, trie_last_dflt)) {
- if (res->fi)
- fib_info_put(res->fi);
- res->fi = fi;
- atomic_inc(&fi->fib_clntref);
+ fib_result_assign(res, fi);
trie_last_dflt = order;
goto out;
}
@@ -1844,20 +1841,12 @@ fn_trie_select_default(struct fib_table *tb, const struct flowi *flp, struct fib
}
if (!fib_detect_death(fi, order, &last_resort, &last_idx, trie_last_dflt)) {
- if (res->fi)
- fib_info_put(res->fi);
- res->fi = fi;
- atomic_inc(&fi->fib_clntref);
+ fib_result_assign(res, fi);
trie_last_dflt = order;
goto out;
}
- if (last_idx >= 0) {
- if (res->fi)
- fib_info_put(res->fi);
- res->fi = last_resort;
- if (last_resort)
- atomic_inc(&last_resort->fib_clntref);
- }
+ if (last_idx >= 0)
+ fib_result_assign(res, last_resort);
trie_last_dflt = last_idx;
out:;
rcu_read_unlock();
--
1.5.3.rc5
^ permalink raw reply related
* [PATCH 2.6.25 1/3] ipv4: no need pass pointer to a default into fib_detect_death
From: Denis V. Lunev @ 2007-12-07 16:09 UTC (permalink / raw)
To: davem; +Cc: containers, devel, netdev
ipv4: no need pass pointer to a default into fib_detect_death
Signed-off-by: Denis V. Lunev <den@openvz.org>
Acked-by: Alexey Kuznetsov <kuznet@ms2.inr.ac.ru>
---
net/ipv4/fib_hash.c | 4 ++--
net/ipv4/fib_lookup.h | 2 +-
net/ipv4/fib_semantics.c | 6 +++---
net/ipv4/fib_trie.c | 4 ++--
4 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/net/ipv4/fib_hash.c b/net/ipv4/fib_hash.c
index 30ff657..76bb7fd 100644
--- a/net/ipv4/fib_hash.c
+++ b/net/ipv4/fib_hash.c
@@ -314,7 +314,7 @@ fn_hash_select_default(struct fib_table *tb, const struct flowi *flp, struct fib
if (next_fi != res->fi)
break;
} else if (!fib_detect_death(fi, order, &last_resort,
- &last_idx, &fn_hash_last_dflt)) {
+ &last_idx, fn_hash_last_dflt)) {
if (res->fi)
fib_info_put(res->fi);
res->fi = fi;
@@ -332,7 +332,7 @@ fn_hash_select_default(struct fib_table *tb, const struct flowi *flp, struct fib
goto out;
}
- if (!fib_detect_death(fi, order, &last_resort, &last_idx, &fn_hash_last_dflt)) {
+ if (!fib_detect_death(fi, order, &last_resort, &last_idx, fn_hash_last_dflt)) {
if (res->fi)
fib_info_put(res->fi);
res->fi = fi;
diff --git a/net/ipv4/fib_lookup.h b/net/ipv4/fib_lookup.h
index eef9eec..6c9dd42 100644
--- a/net/ipv4/fib_lookup.h
+++ b/net/ipv4/fib_lookup.h
@@ -36,6 +36,6 @@ extern struct fib_alias *fib_find_alias(struct list_head *fah,
u8 tos, u32 prio);
extern int fib_detect_death(struct fib_info *fi, int order,
struct fib_info **last_resort,
- int *last_idx, int *dflt);
+ int *last_idx, int dflt);
#endif /* _FIB_LOOKUP_H */
diff --git a/net/ipv4/fib_semantics.c b/net/ipv4/fib_semantics.c
index ec9b0dd..bbd4a24 100644
--- a/net/ipv4/fib_semantics.c
+++ b/net/ipv4/fib_semantics.c
@@ -346,7 +346,7 @@ struct fib_alias *fib_find_alias(struct list_head *fah, u8 tos, u32 prio)
}
int fib_detect_death(struct fib_info *fi, int order,
- struct fib_info **last_resort, int *last_idx, int *dflt)
+ struct fib_info **last_resort, int *last_idx, int dflt)
{
struct neighbour *n;
int state = NUD_NONE;
@@ -358,10 +358,10 @@ int fib_detect_death(struct fib_info *fi, int order,
}
if (state==NUD_REACHABLE)
return 0;
- if ((state&NUD_VALID) && order != *dflt)
+ if ((state&NUD_VALID) && order != dflt)
return 0;
if ((state&NUD_VALID) ||
- (*last_idx<0 && order > *dflt)) {
+ (*last_idx<0 && order > dflt)) {
*last_resort = fi;
*last_idx = order;
}
diff --git a/net/ipv4/fib_trie.c b/net/ipv4/fib_trie.c
index 6385cca..914a0d2 100644
--- a/net/ipv4/fib_trie.c
+++ b/net/ipv4/fib_trie.c
@@ -1827,7 +1827,7 @@ fn_trie_select_default(struct fib_table *tb, const struct flowi *flp, struct fib
if (next_fi != res->fi)
break;
} else if (!fib_detect_death(fi, order, &last_resort,
- &last_idx, &trie_last_dflt)) {
+ &last_idx, trie_last_dflt)) {
if (res->fi)
fib_info_put(res->fi);
res->fi = fi;
@@ -1843,7 +1843,7 @@ fn_trie_select_default(struct fib_table *tb, const struct flowi *flp, struct fib
goto out;
}
- if (!fib_detect_death(fi, order, &last_resort, &last_idx, &trie_last_dflt)) {
+ if (!fib_detect_death(fi, order, &last_resort, &last_idx, trie_last_dflt)) {
if (res->fi)
fib_info_put(res->fi);
res->fi = fi;
--
1.5.3.rc5
^ permalink raw reply related
* IPsec replay sequence number overflow behavior? (RFC4303 section 3.3.3)
From: Paul Moore @ 2007-12-07 16:04 UTC (permalink / raw)
To: netdev; +Cc: Joy Latten
Hello all,
As part of the IPv6 "gap analysis" that the Linux Foundation is currently
doing I've been looking at the IPsec auditing requirements as defined in
RFC4303 and I came across some odd behavior regarding SA sequence number
overflows ...
RFC4303 states the following:
3.3.3. Sequence Number Generation
The sender's counter is initialized to 0 when an SA is established.
The sender increments the sequence number (or ESN) counter for this
SA and inserts the low-order 32 bits of the value into the Sequence
Number field. Thus, the first packet sent using a given SA will
contain a sequence number of 1.
If anti-replay is enabled (the default), the sender checks to ensure
that the counter has not cycled before inserting the new value in the
Sequence Number field. In other words, the sender MUST NOT send a
packet on an SA if doing so would cause the sequence number to cycle.
An attempt to transmit a packet that would result in sequence number
overflow is an auditable event. The audit log entry for this event
SHOULD include the SPI value, current date/time, Source Address,
Destination Address, and (in IPv6) the cleartext Flow ID.
The related code in net/xfrm/xfrm_output.c:xfrm_output() looks like this:
if (x->type->flags & XFRM_TYPE_REPLAY_PROT) {
XFRM_SKB_CB(skb)->seq = ++x->replay.oseq;
if (xfrm_aevent_is_on())
xfrm_replay_notify(x, XFRM_REPLAY_UPDATE);
}
Which doesn't appear to take into account sequence number overflow at all.
Granted, it does send notifications to userspace but it doesn't do anything
to prevent the packet from being sent if the sequence number wraps. I'm
still a few years behind in my IPsec specifications so I could be missing
something here (extended sequence numbers spring to mind and the kernel's
curious mixing of 32bit and 64bit types for SA sequence number counters) but
at first glance this appears to be a bug ... yes/no?
If it is a bug, I think the basic fix should be pretty simple, changing the
above xfrm_output() code to the following:
if (x->type->flags & XFRM_TYPE_REPLAY_PROT) {
XFRM_SKB_CB(skb)->seq = ++x->replay.oseq;
+ if (x->replay.oseq == 0)
+ goto error;
if (xfrm_aevent_is_on())
xfrm_replay_notify(x, XFRM_REPLAY_UPDATE);
}
--
paul moore
linux security @ hp
^ permalink raw reply
* Re: [patch 07/22] NET: DM9000: Use msleep() instead of udelay()
From: Ben Dooks @ 2007-12-07 15:42 UTC (permalink / raw)
To: Jeff Garzik; +Cc: Ben Dooks, netdev, vince
In-Reply-To: <474780E1.2020700@garzik.org>
On Fri, Nov 23, 2007 at 08:39:45PM -0500, Jeff Garzik wrote:
> are you sure you cannot sleep during suspend?
Yes. This is not the first driver that has had this problem,
see the sm501 as another example.
--
Ben (ben@fluff.org, http://www.fluff.org/)
'a smiley only costs 4 bytes'
^ permalink raw reply
* Re: [Patch] net/xfrm/xfrm_policy.c: Some small improvements
From: Richard Knutsson @ 2007-12-07 15:41 UTC (permalink / raw)
To: David Miller; +Cc: xiyou.wangcong, linux-kernel, herbert, akpm, netdev
In-Reply-To: <20071206.192249.193354742.davem@davemloft.net>
David Miller wrote:
> From: Richard Knutsson <ricknu-0@student.ltu.se>
> Date: Thu, 06 Dec 2007 15:37:46 +0100
>
>
>> David Miller wrote:
>>
>>> But this time I'll just let you know up front that I
>>> don't see much value in this patch. It is not a clear
>>> improvement to replace int's with bool's in my mind and
>>> the other changes are just whitespace changes.
>>>
>>>
>> Is it not an improvement to distinct booleans from actual values? Do you
>> use integers for ASCII characters too? It can also avoid some potential
>> bugs like the 'if (i == TRUE)'...
>> What is wrong with 'size_t' (since it is unsigned, compared to (some)
>> 'int')?
>>
>
> When you say "int found;" is there any doubt in your mind that
> this integer is going to hold a 1 or a 0 depending upon whether
> we "found" something?
>
> That's the problem I have with these kinds of patches, they do
> not increase clarity, it's just pure mindless edits.
>
But is there not a good thing if also the compiler knows + names are
sometime not as clear as that one?
> In new code, fine, use booleans if you want.
>
> I would even accept that it helps to change to boolean for
> arguments to functions that are global in scope.
>
> But not for function local variables in cases like this.
>
Oh, I see your point now. Believed it to be yet another 'booleans is not
C idiom'.
Sorry about the noise
Richard Knutsson
^ permalink raw reply
* Re: [patch 0/3][IPV6]: remove ifdef in route6 init/fini functions
From: YOSHIFUJI Hideaki / 吉藤英明 @ 2007-12-07 15:37 UTC (permalink / raw)
To: dlezcano, davem; +Cc: pekkas, netdev, yoshfuji
In-Reply-To: <20071207131325.332768305@ICON-9-164-138-215.megacenter.de.ibm.com>
In article <20071207131325.332768305@ICON-9-164-138-215.megacenter.de.ibm.com> (at Fri, 07 Dec 2007 14:13:25 +0100), Daniel Lezcano <dlezcano@fr.ibm.com> says:
> The route6 init function is a little difficult to read because it contains
> a lot of ifdef. The patchset redefines the usual static inline functions when
> the code is to be disabled by configuration, so we can call the code without
> taking care of the config option in the init function.
Acked-by: YOSHIFUJI Hideaki <yoshfuji@linux-ipv6.org>
--yoshfuji
^ permalink raw reply
* [patch 1/3][IPV6]: create route6 proc init-fini functions
From: Daniel Lezcano @ 2007-12-07 13:13 UTC (permalink / raw)
To: davem; +Cc: yoshfuji, pekkas, netdev
In-Reply-To: <20071207131325.332768305@ICON-9-164-138-215.megacenter.de.ibm.com>
[-- Attachment #1: route6-make-a-specific-proc-init-fini-function.patch --]
[-- Type: text/plain, Size: 2640 bytes --]
Make the proc creation/destruction to be a separate function. That allows to
remove the #ifdef CONFIG_PROC_FS in the init/fini function and make them more
readable.
Signed-off-by: Daniel Lezcano <dlezcano@fr.ibm.com>
---
net/ipv6/route.c | 58 +++++++++++++++++++++++++++++++++++++------------------
1 file changed, 40 insertions(+), 18 deletions(-)
Index: net-2.6.25/net/ipv6/route.c
===================================================================
--- net-2.6.25.orig/net/ipv6/route.c
+++ net-2.6.25/net/ipv6/route.c
@@ -2353,6 +2353,40 @@ static const struct file_operations rt6_
.llseek = seq_lseek,
.release = single_release,
};
+
+static int ipv6_route_proc_init(struct net *net)
+{
+ int ret = -ENOMEM;
+ if (!proc_net_fops_create(net, "ipv6_route",
+ 0, &ipv6_route_proc_fops))
+ goto out;
+
+ if (!proc_net_fops_create(net, "rt6_stats",
+ S_IRUGO, &rt6_stats_seq_fops))
+ goto out_ipv6_route;
+
+ ret = 0;
+out:
+ return ret;
+out_ipv6_route:
+ proc_net_remove(net, "ipv6_route");
+ goto out;
+}
+
+static void ipv6_route_proc_fini(struct net *net)
+{
+ proc_net_remove(net, "ipv6_route");
+ proc_net_remove(net, "rt6_stats");
+}
+#else
+static inline int ipv6_route_proc_init(struct net *net)
+{
+ return 0;
+}
+static inline void ipv6_route_proc_fini(struct net *net)
+{
+ return ;
+}
#endif /* CONFIG_PROC_FS */
#ifdef CONFIG_SYSCTL
@@ -2479,21 +2513,14 @@ int __init ip6_route_init(void)
if (ret)
goto out_kmem_cache;
-#ifdef CONFIG_PROC_FS
- ret = -ENOMEM;
- if (!proc_net_fops_create(&init_net, "ipv6_route",
- 0, &ipv6_route_proc_fops))
+ ret = ipv6_route_proc_init(&init_net);
+ if (ret)
goto out_fib6_init;
- if (!proc_net_fops_create(&init_net, "rt6_stats",
- S_IRUGO, &rt6_stats_seq_fops))
- goto out_proc_ipv6_route;
-#endif
-
#ifdef CONFIG_XFRM
ret = xfrm6_init();
if (ret)
- goto out_proc_rt6_stats;
+ goto out_proc_init;
#endif
#ifdef CONFIG_IPV6_MULTIPLE_TABLES
ret = fib6_rules_init();
@@ -2517,14 +2544,10 @@ xfrm6_init:
#endif
#ifdef CONFIG_XFRM
xfrm6_fini();
-out_proc_rt6_stats:
#endif
-#ifdef CONFIG_PROC_FS
- proc_net_remove(&init_net, "rt6_stats");
-out_proc_ipv6_route:
- proc_net_remove(&init_net, "ipv6_route");
+out_proc_init:
+ ipv6_route_proc_fini(&init_net);
out_fib6_init:
-#endif
rt6_ifdown(NULL);
fib6_gc_cleanup();
out_kmem_cache:
@@ -2537,8 +2560,7 @@ void ip6_route_cleanup(void)
#ifdef CONFIG_IPV6_MULTIPLE_TABLES
fib6_rules_cleanup();
#endif
- proc_net_remove(&init_net, "ipv6_route");
- proc_net_remove(&init_net, "rt6_stats");
+ ipv6_route_proc_fini(&init_net);
#ifdef CONFIG_XFRM
xfrm6_fini();
#endif
--
^ permalink raw reply
* [patch 0/3][IPV6]: remove ifdef in route6 init/fini functions
From: Daniel Lezcano @ 2007-12-07 13:13 UTC (permalink / raw)
To: davem; +Cc: yoshfuji, pekkas, netdev
The route6 init function is a little difficult to read because it contains
a lot of ifdef. The patchset redefines the usual static inline functions when
the code is to be disabled by configuration, so we can call the code without
taking care of the config option in the init function.
--
^ permalink raw reply
* [patch 2/3][IPV6]: remove ifdef in route6 for xfrm6
From: Daniel Lezcano @ 2007-12-07 13:13 UTC (permalink / raw)
To: davem; +Cc: yoshfuji, pekkas, netdev
In-Reply-To: <20071207131325.332768305@ICON-9-164-138-215.megacenter.de.ibm.com>
[-- Attachment #1: route6-remove-ifdef-for-xfrm-init.patch --]
[-- Type: text/plain, Size: 2399 bytes --]
The following patch create the usual static inline functions to disable
the xfrm6_init and xfrm6_fini function when XFRM is off.
That's allow to remove some ifdef and make the code a little more clear.
Signed-off-by: Daniel Lezcano <dlezcano@fr.ibm.com>
---
include/net/xfrm.h | 16 +++++++++++++---
net/ipv6/route.c | 7 +------
2 files changed, 14 insertions(+), 9 deletions(-)
Index: net-2.6.25/include/net/xfrm.h
===================================================================
--- net-2.6.25.orig/include/net/xfrm.h
+++ net-2.6.25/include/net/xfrm.h
@@ -842,7 +842,6 @@ xfrm_state_addr_cmp(struct xfrm_tmpl *tm
}
#ifdef CONFIG_XFRM
-
extern int __xfrm_policy_check(struct sock *, int dir, struct sk_buff *skb, unsigned short family);
static inline int xfrm_policy_check(struct sock *sk, int dir, struct sk_buff *skb, unsigned short family)
@@ -1066,12 +1065,23 @@ struct xfrm6_tunnel {
extern void xfrm_init(void);
extern void xfrm4_init(void);
-extern int xfrm6_init(void);
-extern void xfrm6_fini(void);
extern void xfrm_state_init(void);
extern void xfrm4_state_init(void);
+#ifdef CONFIG_XFRM
+extern int xfrm6_init(void);
+extern void xfrm6_fini(void);
extern int xfrm6_state_init(void);
extern void xfrm6_state_fini(void);
+#else
+static inline int xfrm6_init(void)
+{
+ return 0;
+}
+static inline void xfrm6_fini(void)
+{
+ ;
+}
+#endif
extern int xfrm_state_walk(u8 proto, int (*func)(struct xfrm_state *, int, void*), void *);
extern struct xfrm_state *xfrm_state_alloc(void);
Index: net-2.6.25/net/ipv6/route.c
===================================================================
--- net-2.6.25.orig/net/ipv6/route.c
+++ net-2.6.25/net/ipv6/route.c
@@ -2517,11 +2517,10 @@ int __init ip6_route_init(void)
if (ret)
goto out_fib6_init;
-#ifdef CONFIG_XFRM
ret = xfrm6_init();
if (ret)
goto out_proc_init;
-#endif
+
#ifdef CONFIG_IPV6_MULTIPLE_TABLES
ret = fib6_rules_init();
if (ret)
@@ -2542,9 +2541,7 @@ fib6_rules_init:
fib6_rules_cleanup();
xfrm6_init:
#endif
-#ifdef CONFIG_XFRM
xfrm6_fini();
-#endif
out_proc_init:
ipv6_route_proc_fini(&init_net);
out_fib6_init:
@@ -2561,9 +2558,7 @@ void ip6_route_cleanup(void)
fib6_rules_cleanup();
#endif
ipv6_route_proc_fini(&init_net);
-#ifdef CONFIG_XFRM
xfrm6_fini();
-#endif
rt6_ifdown(NULL);
fib6_gc_cleanup();
kmem_cache_destroy(ip6_dst_ops.kmem_cachep);
--
^ permalink raw reply
* [patch 3/3][IPV6]: route6 remove ifdef for fib_rules
From: Daniel Lezcano @ 2007-12-07 13:13 UTC (permalink / raw)
To: davem; +Cc: yoshfuji, pekkas, netdev
In-Reply-To: <20071207131325.332768305@ICON-9-164-138-215.megacenter.de.ibm.com>
[-- Attachment #1: route6-remove-ifdef-for-multiple-tables.patch --]
[-- Type: text/plain, Size: 1911 bytes --]
The patch defines the usual static inline functions when the code is
disabled for fib6_rules. That's allow to remove some ifdef in route.c
file and make the code a little more clear.
Signed-off-by: Daniel Lezcano <dlezcano@fr.ibm.com>
---
include/net/ip6_fib.h | 12 +++++++++++-
net/ipv6/route.c | 7 +------
2 files changed, 12 insertions(+), 7 deletions(-)
Index: net-2.6.25/include/net/ip6_fib.h
===================================================================
--- net-2.6.25.orig/include/net/ip6_fib.h
+++ net-2.6.25/include/net/ip6_fib.h
@@ -226,8 +226,18 @@ extern void fib6_gc_cleanup(void);
extern int fib6_init(void);
+#ifdef CONFIG_IPV6_MULTIPLE_TABLES
extern int fib6_rules_init(void);
extern void fib6_rules_cleanup(void);
-
+#else
+static inline int fib6_rules_init(void)
+{
+ return 0;
+}
+static inline void fib6_rules_cleanup(void)
+{
+ return ;
+}
+#endif
#endif
#endif
Index: net-2.6.25/net/ipv6/route.c
===================================================================
--- net-2.6.25.orig/net/ipv6/route.c
+++ net-2.6.25/net/ipv6/route.c
@@ -2521,11 +2521,10 @@ int __init ip6_route_init(void)
if (ret)
goto out_proc_init;
-#ifdef CONFIG_IPV6_MULTIPLE_TABLES
ret = fib6_rules_init();
if (ret)
goto xfrm6_init;
-#endif
+
ret = -ENOBUFS;
if (__rtnl_register(PF_INET6, RTM_NEWROUTE, inet6_rtm_newroute, NULL) ||
__rtnl_register(PF_INET6, RTM_DELROUTE, inet6_rtm_delroute, NULL) ||
@@ -2537,10 +2536,8 @@ out:
return ret;
fib6_rules_init:
-#ifdef CONFIG_IPV6_MULTIPLE_TABLES
fib6_rules_cleanup();
xfrm6_init:
-#endif
xfrm6_fini();
out_proc_init:
ipv6_route_proc_fini(&init_net);
@@ -2554,9 +2551,7 @@ out_kmem_cache:
void ip6_route_cleanup(void)
{
-#ifdef CONFIG_IPV6_MULTIPLE_TABLES
fib6_rules_cleanup();
-#endif
ipv6_route_proc_fini(&init_net);
xfrm6_fini();
rt6_ifdown(NULL);
--
^ permalink raw reply
* Re: 2.6.24-rc4-mm1
From: Ilpo Järvinen @ 2007-12-07 13:16 UTC (permalink / raw)
To: reuben-linuxkernel; +Cc: akpm, LKML, Netdev, auke-jan.h.kok, David Miller
In-Reply-To: <20071205.230925.99250129.davem@davemloft.net>
On Wed, 5 Dec 2007, David Miller wrote:
> From: Reuben Farrelly <reuben-linuxkernel@reub.net>
> Date: Thu, 06 Dec 2007 17:59:37 +1100
>
> > On 5/12/2007 4:17 PM, Andrew Morton wrote:
> > > - Lots of device IDs have been removed from the e1000 driver and moved over
> > > to e1000e. So if your e1000 stops working, you forgot to set CONFIG_E1000E.
> >
> > This non fatal oops which I have just noticed may be related to this change then
> > - certainly looks networking related.
> >
> > WARNING: at net/ipv4/tcp_input.c:2518 tcp_fastretrans_alert()
> > Pid: 0, comm: swapper Not tainted 2.6.24-rc4-mm1 #1
> >
> > Call Trace:
> > <IRQ> [<ffffffff8046e038>] tcp_fastretrans_alert+0x229/0xe63
> > [<ffffffff80470975>] tcp_ack+0xa3f/0x127d
> > [<ffffffff804747b7>] tcp_rcv_established+0x55f/0x7f8
> > [<ffffffff8047b1aa>] tcp_v4_do_rcv+0xdb/0x3a7
> > [<ffffffff881148a8>] :nf_conntrack:nf_ct_deliver_cached_events+0x75/0x99
>
> No, it's from TCP assertions and changes added by Ilpo to the
> net-2.6.25 tree recently.
Yeah, this (very likely) due to the new SACK processing (in net-2.6.25).
I'll look what could go wrong with fack_count calculations, most likely
it's the reason (I've found earlier one out-of-place retransmission
segment in one of my test case which already indicated that there's
something incorrect with them but didn't have time to debug it yet).
Thanks for report. Some info about how easily you can reproduce &
couple of sentences about the test case might be useful later on when
evaluating the fix.
--
i.
^ 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