* Re: [PATCH] xfrm: do not check x->km.state
From: RongQing Li @ 2012-12-14 7:02 UTC (permalink / raw)
To: David Miller; +Cc: steffen.klassert, netdev
In-Reply-To: <20121213.141922.2249665409902614569.davem@davemloft.net>
2012/12/14 David Miller <davem@davemloft.net>:
> From: Steffen Klassert <steffen.klassert@secunet.com>
> Date: Thu, 13 Dec 2012 11:19:48 +0100
>
>> On Thu, Dec 13, 2012 at 05:06:00PM +0800, roy.qing.li@gmail.com wrote:
>>> From: Li RongQing <roy.qing.li@gmail.com>
>>>
>>> do not check x->km.state, it will be checked by succedent
>>> xfrm_state_check_expire()
>>>
>>> Signed-off-by: Li RongQing <roy.qing.li@gmail.com>
> ...
>> This would remove the only place where the LINUX_MIB_XFRMINSTATEINVALID
>> statistics counter is incremented. I think it would be better to ensure
>> a valid state before we call xfrm_state_check_expire(). This would make
>> the statistics more accurate and we can remove the x->km.state check
>> from xfrm_state_check_expire().
>
> Agreed.
Thanks.
since xfrm_output_one() calls xfrm_state_check_expire() too, but without
checking (x->km.state != XFRM_STATE_VALID), I think we can not directly
remove the check of km.state from xfrm_state_check_expire(). I have two
option, which one do you think it is better?
1. remove this check in xfrm_state_check_expire, and add a check in
xfrm_output_one
diff --git a/net/xfrm/xfrm_output.c b/net/xfrm/xfrm_output.c
index 95a338c..c245370 100644
--- a/net/xfrm/xfrm_output.c
+++ b/net/xfrm/xfrm_output.c
@@ -61,6 +61,12 @@ static int xfrm_output_one(struct sk_buff *skb, int err)
}
spin_lock_bh(&x->lock);
+
+ if (unlikely(x->km.state != XFRM_STATE_VALID)) {
+ XFRM_INC_STATS(net, LINUX_MIB_XFRMOUTSTATEINVALID);
+ goto drop_unlock;
+ }
+
err = xfrm_state_check_expire(x);
if (err) {
XFRM_INC_STATS(net, LINUX_MIB_XFRMOUTSTATEEXPIRED);
diff --git a/net/xfrm/xfrm_proc.c b/net/xfrm/xfrm_proc.c
index d0a1af8..e4cd441 100644
--- a/net/xfrm/xfrm_proc.c
+++ b/net/xfrm/xfrm_proc.c
@@ -39,6 +39,7 @@ static const struct snmp_mib xfrm_mib_list[] = {
SNMP_MIB_ITEM("XfrmOutStateModeError", LINUX_MIB_XFRMOUTSTATEMODEERROR),
SNMP_MIB_ITEM("XfrmOutStateSeqError", LINUX_MIB_XFRMOUTSTATESEQERROR),
SNMP_MIB_ITEM("XfrmOutStateExpired", LINUX_MIB_XFRMOUTSTATEEXPIRED),
+ SNMP_MIB_ITEM("XfrmOutStateInvalid", LINUX_MIB_XFRMOUTSTATEINVALID),
SNMP_MIB_ITEM("XfrmOutPolBlock", LINUX_MIB_XFRMOUTPOLBLOCK),
SNMP_MIB_ITEM("XfrmOutPolDead", LINUX_MIB_XFRMOUTPOLDEAD),
SNMP_MIB_ITEM("XfrmOutPolError", LINUX_MIB_XFRMOUTPOLERROR),
diff --git a/net/xfrm/xfrm_state.c b/net/xfrm/xfrm_state.c
index 3459692..05db236 100644
--- a/net/xfrm/xfrm_state.c
+++ b/net/xfrm/xfrm_state.c
@@ -1370,9 +1370,6 @@ int xfrm_state_check_expire(struct xfrm_state *x)
if (!x->curlft.use_time)
x->curlft.use_time = get_seconds();
- if (x->km.state != XFRM_STATE_VALID)
- return -EINVAL;
-
if (x->curlft.bytes >= x->lft.hard_byte_limit ||
x->curlft.packets >= x->lft.hard_packet_limit) {
x->km.state = XFRM_STATE_EXPIRED;
2. Only remove this check in xfrm6_input.c
--- a/net/ipv6/xfrm6_input.c
+++ b/net/ipv6/xfrm6_input.c
@@ -109,7 +109,6 @@ int xfrm6_input_addr(struct sk_buff *skb,
xfrm_address_t *daddr,
if ((!i || (x->props.flags & XFRM_STATE_WILDRECV)) &&
- likely(x->km.state == XFRM_STATE_VALID) &&
!xfrm_state_check_expire(x)) {
spin_unlock(&x->lock);
if (x->type->input(x, skb) > 0) {
/* found a valid state */
^ permalink raw reply related
* Re: [RFC PATCH] xfrm: avoid to send/receive the exceeding hard lifetime data
From: RongQing Li @ 2012-12-14 6:58 UTC (permalink / raw)
To: Steffen Klassert; +Cc: netdev
In-Reply-To: <20121213101422.GF18940@secunet.com>
2012/12/13 Steffen Klassert <steffen.klassert@secunet.com>:
> On Thu, Dec 13, 2012 at 04:25:52PM +0800, roy.qing.li@gmail.com wrote:
>> From: Li RongQing <roy.qing.li@gmail.com>
>>
>> If setkey sets both bh and bs as 1024, and the total send and receive package
>> size is 1024, then if next package size is too large, this package should be
>> discard.
>>
>> Example, first package size is 1000, send success, then the second package
>> is 500, 1000+500 is larger than 1024, so the second package should be discard.
>>
>> Signed-off-by: Li RongQing <roy.qing.li@gmail.com>
>> ---
>> net/xfrm/xfrm_input.c | 6 +++---
>> net/xfrm/xfrm_output.c | 6 +++---
>> 2 files changed, 6 insertions(+), 6 deletions(-)
>>
>> diff --git a/net/xfrm/xfrm_input.c b/net/xfrm/xfrm_input.c
>> index ab2bb42..d0de8f3 100644
>> --- a/net/xfrm/xfrm_input.c
>> +++ b/net/xfrm/xfrm_input.c
>> @@ -178,6 +178,9 @@ int xfrm_input(struct sk_buff *skb, int nexthdr, __be32 spi, int encap_type)
>> goto drop_unlock;
>> }
>>
>> + x->curlft.bytes += skb->len;
>> + x->curlft.packets++;
>> +
>
> This is a bit critical on input. We should only increment these values
> if the integrity check on this packet was successfull. Otherwise someone
> could spam us with invalid packets and trigger a state expiry.
>
> If a synchronous crypto algorithm is used, we send at most one packet too
> much. The maximal byte count was not yet reached and RFC 2401 says not
> much on how to handle the packet that reaches the maximal byte count,
> so this is probaply ok.
>
Yes, RFC does not say how to handle this packet.
But when I do a IPsec compliance test with IxANVL, the test case 5.3/5.11,
which reports a error because it expects this packet should be dropped, but not.
I do not know if it is bug, or if it is valuable to fix it?
-Li
> But if an asynchronous crypto algorithm is used, we can send a lot
> of packets too much. So we should probaply add a second expiry check
> after resume from asynchronous crypto. We do this already with the replay
> check.
>
^ permalink raw reply
* [PATCH] add a `make dist` helper
From: Mike Frysinger @ 2012-12-14 6:34 UTC (permalink / raw)
To: stephen.hemminger, netdev
This makes sure the tarball is always created in the same way.
Avoids accidental typos in path names for example.
Signed-off-by: Mike Frysinger <vapier@gentoo.org>
---
.gitignore | 1 +
Makefile | 10 ++++++++++
2 files changed, 11 insertions(+)
diff --git a/.gitignore b/.gitignore
index 3ba2632..e4490f4 100644
--- a/.gitignore
+++ b/.gitignore
@@ -1,3 +1,4 @@
+iproute2-*.tar*
static-syms.h
config.*
Config
diff --git a/Makefile b/Makefile
index 46a5ad9..198abea 100644
--- a/Makefile
+++ b/Makefile
@@ -75,6 +75,16 @@ clobber:
distclean: clobber
+GIT_ARCHIVE = git archive --prefix=iproute2-$(VER)/ v$(VER) | $(1) > iproute2-$(VER).tar.$(2)
+dist.gz: ; $(call GIT_ARCHIVE,gzip,gz)
+dist.bz2: ; $(call GIT_ARCHIVE,bzip2,bz2)
+dist.xz: ; $(call GIT_ARCHIVE,xz,xz)
+dist:
+ifeq ($(VER),)
+ @echo "Usage: make dist VER=3.7.0"; false
+endif
+ $(MAKE) dist.gz dist.bz2 dist.xz
+
cscope:
cscope -b -q -R -Iinclude -sip -slib -smisc -snetem -stc
--
1.8.0
^ permalink raw reply related
* [Patch] iproute2: clean up genl/genl.c::usage()
From: Cong Wang @ 2012-12-14 6:10 UTC (permalink / raw)
To: netdev; +Cc: Stephen Hemminger, Cong Wang
gcc attribute can be used in definition, no need to
redeclare it.
Cc: Stephen Hemminger <shemminger@vyatta.com>
Signed-off-by: Cong Wang <amwang@redhat.com>
---
diff --git a/genl/genl.c b/genl/genl.c
index 49b6596..a1e55e0 100644
--- a/genl/genl.c
+++ b/genl/genl.c
@@ -97,9 +97,7 @@ noexist:
return f;
}
-static void usage(void) __attribute__((noreturn));
-
-static void usage(void)
+static void __attribute__((noreturn)) usage(void)
{
fprintf(stderr, "Usage: genl [ OPTIONS ] OBJECT | help }\n"
"where OBJECT := { ctrl etc }\n"
^ permalink raw reply related
* [PATCH] iproute2: update usage info of bridge monitor
From: Cong Wang @ 2012-12-14 5:59 UTC (permalink / raw)
To: netdev; +Cc: Stephen Hemminger, bridge, Cong Wang
Cc: Stephen Hemminger <shemminger@vyatta.com>
Signed-off-by: Cong Wang <amwang@redhat.com>
---
diff --git a/bridge/monitor.c b/bridge/monitor.c
index 44e14d8..29bb931 100644
--- a/bridge/monitor.c
+++ b/bridge/monitor.c
@@ -31,7 +31,7 @@ int prefix_banner;
static void usage(void)
{
- fprintf(stderr, "Usage: bridge monitor\n");
+ fprintf(stderr, "Usage: bridge monitor [file | link | fdb | mdb | all]\n");
exit(-1);
}
^ permalink raw reply related
* Re: [RFC PATCH net-next 4/4 V4] try to fix performance regression
From: Weiping Pan @ 2012-12-14 5:53 UTC (permalink / raw)
To: Rick Jones; +Cc: David Laight, davem, brutus, netdev
In-Reply-To: <50CA1DAB.5050000@hp.com>
On 12/14/2012 02:25 AM, Rick Jones wrote:
> On 12/13/2012 06:05 AM, Weiping Pan wrote:
>> But if I just run normal tcp loopback for each message size, then the
>> performance is stable.
>> [root@intel-s3e3432-01 ~]# cat base.sh
>> for s in 1 2 4 8 16 32 64 128 256 512 1024 2048 4096 8192 16384 32768
>> 65536 131072 262144 524288 1048576
>> do
>> netperf -i -2,10 -I 95,20 -- -m $s -M $s | tail -n1
>> done
>
> The -i option goes max,min iterations:
>
> http://www.netperf.org/svn/netperf2/trunk/doc/netperf.html#index-g_t_002di_002c-Global-28
>
>
> and src/netsh.c will apply some silent clipping to that:
>
>
> case 'i':
> /* set the iterations min and max for confidence intervals */
> break_args(optarg,arg1,arg2);
> if (arg1[0]) {
> iteration_max = convert(arg1);
> }
> if (arg2[0] ) {
> iteration_min = convert(arg2);
> }
> /* if the iteration_max is < iteration_min make iteration_max
> equal iteration_min */
> if (iteration_max < iteration_min) iteration_max = iteration_min;
> /* limit minimum to 3 iterations */
> if (iteration_max < 3) iteration_max = 3;
> if (iteration_min < 3) iteration_min = 3;
> /* limit maximum to 30 iterations */
> if (iteration_max > 30) iteration_max = 30;
> if (iteration_min > 30) iteration_min = 30;
> if (confidence_level == 0) confidence_level = 99;
> if (interval == 0.0) interval = 0.05; /* five percent */
> break;
>
> So, what will happen with your netperf command line above is it will
> set iteration max to 10 iterations and it will always run 10
> iterations since min will equal max. If you want it to possibly
> terminate sooner upon hitting the confidence intervals you would want
> to go with -i 10,3. That will have netperf always run at least three
> and no more than 10 iterations.
Yes, I misread the manual, it should be "-i 10,3".
>
> If I'm not mistaken, the use of the "| tail -n 1" there will cause the
> "classic" confidence intervals not met warning to be tossed (unless I
> suppose it is actually going to stderr?).
Yes, I saw that warning.
>
> If you use the "omni" tests directly rather than via "migration" you
> will no longer get warnings about not hitting the confidence interval,
> but you can have netperf emit the confidence level it actually
> achieved as well as the number of iterations it took to get there.
> You would use the omni output selection to do that.
>
> http://www.netperf.org/svn/netperf2/trunk/doc/netperf.html#Omni-Output-Selection
>
>
>
> These may have been mentioned before...
>
> Judging from that command line you have the potential variability of
> the socket buffer auto-tuning. Does AF_UNIX do the same sort of auto
> tuning? It may be desirable to add some test-specific -s and -S
> options to have a fixed socket buffer size.
I set -s 51882 -m 16384 -M 87380 for all the three kinds of sockets by
default.
>
> Since the MTU for loopback is ~16K, the send sizes below that will
> probably have differing interactions with the Nagle algorithm.
> Particularly as I suspect the timing will differ between friends and
> no friends.
>
> I would guess the most "consistent" comparison with AF_UNIX would be
> when Nagle is disabled for the TCP_STREAM tests. That would be a
> test-specific -D option.
>
> Perhaps a more "stable" way to compare friends, no-friends and unix
> would be to use the _RR tests. That will be a more direct, less-prone
> to other heuristics measure of path-length differences - both in the
> reported transactions per second and in any CPU utilization/service
> demand if you enable that via -c. I'm not sure it would be necessary
> to take the request/response size out beyond a couple KB. Take it out
> to the MB level and you will probably return to the question of
> auto-tuning of the socket buffer sizes.
Good suggestion !
>
> happy benchmarking,
>
> rick jones
Rick, thanks !
Weiping Pan
^ permalink raw reply
* [PATCH V2] ndisc: Use more standard logging styles
From: Joe Perches @ 2012-12-14 4:23 UTC (permalink / raw)
To: David S. Miller, Alexey Kuznetsov, James Morris,
Hideaki YOSHIFUJI, Patrick McHardy
Cc: Duan Jiong, Steffen Klassert, netdev, linux-kernel
In-Reply-To: <1355458079.29363.4.camel@joe-AO722>
The logging style in this file is "baroque".
Convert indirect uses of net_<level>_ratelimited to
simply use net_<level>_ratelimited.
Add a nd_dbg macro and #define ND_DEBUG for the other
generally inactivated logging messages.
Make those inactivated messages emit only at KERN_DEBUG
instead of other levels. Add "%s: " __func__ to all
these nd_dbg macros and remove the embedded function
names and prefixes.
Signed-off-by: Joe Perches <joe@perches.com>
---
net/ipv6/ndisc.c | 139 ++++++++++++++++++++++++------------------------------
1 files changed, 61 insertions(+), 78 deletions(-)
diff --git a/net/ipv6/ndisc.c b/net/ipv6/ndisc.c
index f2a007b..de1c1f2 100644
--- a/net/ipv6/ndisc.c
+++ b/net/ipv6/ndisc.c
@@ -72,14 +72,19 @@
#include <linux/netfilter.h>
#include <linux/netfilter_ipv6.h>
-/* Set to 3 to get tracing... */
-#define ND_DEBUG 1
+/* #define ND_DEBUG */
-#define ND_PRINTK(val, level, fmt, ...) \
+#ifdef ND_DEBUG
+#define nd_dbg(fmt, ...) \
+ net_dbg_ratelimited("%s: " fmt, __func__, ##__VA_ARGS__)
+#else
+#define nd_dbg(fmt, ...) \
do { \
- if (val <= ND_DEBUG) \
- net_##level##_ratelimited(fmt, ##__VA_ARGS__); \
+ if (0) \
+ net_dbg_ratelimited("%s: " fmt, \
+ __func__, ##__VA_ARGS__); \
} while (0)
+#endif
static u32 ndisc_hash(const void *pkey,
const struct net_device *dev,
@@ -220,9 +225,8 @@ struct ndisc_options *ndisc_parse_options(u8 *opt, int opt_len,
case ND_OPT_MTU:
case ND_OPT_REDIRECT_HDR:
if (ndopts->nd_opt_array[nd_opt->nd_opt_type]) {
- ND_PRINTK(2, warn,
- "%s: duplicated ND6 option found: type=%d\n",
- __func__, nd_opt->nd_opt_type);
+ nd_dbg("duplicated ND6 option found: type=%d\n",
+ nd_opt->nd_opt_type);
} else {
ndopts->nd_opt_array[nd_opt->nd_opt_type] = nd_opt;
}
@@ -250,11 +254,9 @@ struct ndisc_options *ndisc_parse_options(u8 *opt, int opt_len,
* to accommodate future extension to the
* protocol.
*/
- ND_PRINTK(2, notice,
- "%s: ignored unsupported option; type=%d, len=%d\n",
- __func__,
- nd_opt->nd_opt_type,
- nd_opt->nd_opt_len);
+ nd_dbg("ignored unsupported option; type=%d, len=%d\n",
+ nd_opt->nd_opt_type,
+ nd_opt->nd_opt_len);
}
}
opt_len -= l;
@@ -399,8 +401,8 @@ static struct sk_buff *ndisc_build_skb(struct net_device *dev,
len + hlen + tlen),
1, &err);
if (!skb) {
- ND_PRINTK(0, err, "ND: %s failed to allocate an skb, err=%d\n",
- __func__, err);
+ net_err_ratelimited("ND: %s failed to allocate an skb, err=%d\n",
+ __func__, err);
return NULL;
}
@@ -629,9 +631,8 @@ static void ndisc_solicit(struct neighbour *neigh, struct sk_buff *skb)
if ((probes -= neigh->parms->ucast_probes) < 0) {
if (!(neigh->nud_state & NUD_VALID)) {
- ND_PRINTK(1, dbg,
- "%s: trying to ucast probe in NUD_INVALID: %pI6\n",
- __func__, target);
+ net_dbg_ratelimited("%s: trying to ucast probe in NUD_INVALID: %pI6\n",
+ __func__, target);
}
ndisc_send_ns(dev, neigh, target, target, saddr);
} else if ((probes -= neigh->parms->app_probes) < 0) {
@@ -677,7 +678,7 @@ static void ndisc_recv_ns(struct sk_buff *skb)
int is_router = -1;
if (ipv6_addr_is_multicast(&msg->target)) {
- ND_PRINTK(2, warn, "NS: multicast target address\n");
+ nd_dbg("multicast target address\n");
return;
}
@@ -690,20 +691,19 @@ static void ndisc_recv_ns(struct sk_buff *skb)
daddr->s6_addr32[1] == htonl(0x00000000) &&
daddr->s6_addr32[2] == htonl(0x00000001) &&
daddr->s6_addr [12] == 0xff )) {
- ND_PRINTK(2, warn, "NS: bad DAD packet (wrong destination)\n");
+ nd_dbg("bad DAD packet (wrong destination)\n");
return;
}
if (!ndisc_parse_options(msg->opt, ndoptlen, &ndopts)) {
- ND_PRINTK(2, warn, "NS: invalid ND options\n");
+ nd_dbg("invalid ND options\n");
return;
}
if (ndopts.nd_opts_src_lladdr) {
lladdr = ndisc_opt_addr_data(ndopts.nd_opts_src_lladdr, dev);
if (!lladdr) {
- ND_PRINTK(2, warn,
- "NS: invalid link-layer address length\n");
+ nd_dbg("invalid link-layer address length\n");
return;
}
@@ -713,8 +713,7 @@ static void ndisc_recv_ns(struct sk_buff *skb)
* in the message.
*/
if (dad) {
- ND_PRINTK(2, warn,
- "NS: bad DAD packet (link-layer address option)\n");
+ nd_dbg("bad DAD packet (link-layer address option)\n");
return;
}
}
@@ -832,30 +831,29 @@ static void ndisc_recv_na(struct sk_buff *skb)
struct neighbour *neigh;
if (skb->len < sizeof(struct nd_msg)) {
- ND_PRINTK(2, warn, "NA: packet too short\n");
+ nd_dbg("packet too short\n");
return;
}
if (ipv6_addr_is_multicast(&msg->target)) {
- ND_PRINTK(2, warn, "NA: target address is multicast\n");
+ nd_dbg("target address is multicast\n");
return;
}
if (ipv6_addr_is_multicast(daddr) &&
msg->icmph.icmp6_solicited) {
- ND_PRINTK(2, warn, "NA: solicited NA is multicasted\n");
+ nd_dbg("solicited NA is multicasted\n");
return;
}
if (!ndisc_parse_options(msg->opt, ndoptlen, &ndopts)) {
- ND_PRINTK(2, warn, "NS: invalid ND option\n");
+ nd_dbg("invalid ND option\n");
return;
}
if (ndopts.nd_opts_tgt_lladdr) {
lladdr = ndisc_opt_addr_data(ndopts.nd_opts_tgt_lladdr, dev);
if (!lladdr) {
- ND_PRINTK(2, warn,
- "NA: invalid link-layer address length\n");
+ nd_dbg("invalid link-layer address length\n");
return;
}
}
@@ -876,9 +874,8 @@ static void ndisc_recv_na(struct sk_buff *skb)
unsolicited advertisement.
*/
if (skb->pkt_type != PACKET_LOOPBACK)
- ND_PRINTK(1, warn,
- "NA: someone advertises our address %pI6 on %s!\n",
- &ifp->addr, ifp->idev->dev->name);
+ net_warn_ratelimited("NA: someone advertises our address %pI6 on %s!\n",
+ &ifp->addr, ifp->idev->dev->name);
in6_ifa_put(ifp);
return;
}
@@ -940,7 +937,7 @@ static void ndisc_recv_rs(struct sk_buff *skb)
idev = __in6_dev_get(skb->dev);
if (!idev) {
- ND_PRINTK(1, err, "RS: can't find in6 device\n");
+ net_err_ratelimited("RS: can't find in6 device\n");
return;
}
@@ -957,7 +954,7 @@ static void ndisc_recv_rs(struct sk_buff *skb)
/* Parse ND options */
if (!ndisc_parse_options(rs_msg->opt, ndoptlen, &ndopts)) {
- ND_PRINTK(2, notice, "NS: invalid ND option, ignored\n");
+ nd_dbg("invalid ND option, ignored\n");
goto out;
}
@@ -1043,17 +1040,17 @@ static void ndisc_router_discovery(struct sk_buff *skb)
optlen = (skb->tail - skb->transport_header) - sizeof(struct ra_msg);
if (!(ipv6_addr_type(&ipv6_hdr(skb)->saddr) & IPV6_ADDR_LINKLOCAL)) {
- ND_PRINTK(2, warn, "RA: source address is not link-local\n");
+ nd_dbg("source address is not link-local\n");
return;
}
if (optlen < 0) {
- ND_PRINTK(2, warn, "RA: packet too short\n");
+ nd_dbg("packet too short\n");
return;
}
#ifdef CONFIG_IPV6_NDISC_NODETYPE
if (skb->ndisc_nodetype == NDISC_NODETYPE_HOST) {
- ND_PRINTK(2, warn, "RA: from host or unauthorized router\n");
+ nd_dbg("from host or unauthorized router\n");
return;
}
#endif
@@ -1064,13 +1061,13 @@ static void ndisc_router_discovery(struct sk_buff *skb)
in6_dev = __in6_dev_get(skb->dev);
if (in6_dev == NULL) {
- ND_PRINTK(0, err, "RA: can't find inet6 device for %s\n",
- skb->dev->name);
+ net_err_ratelimited("RA: can't find inet6 device for %s\n",
+ skb->dev->name);
return;
}
if (!ndisc_parse_options(opt, optlen, &ndopts)) {
- ND_PRINTK(2, warn, "RA: invalid ND options\n");
+ nd_dbg("invalid ND options\n");
return;
}
@@ -1123,9 +1120,8 @@ static void ndisc_router_discovery(struct sk_buff *skb)
if (rt) {
neigh = dst_neigh_lookup(&rt->dst, &ipv6_hdr(skb)->saddr);
if (!neigh) {
- ND_PRINTK(0, err,
- "RA: %s got default router without neighbour\n",
- __func__);
+ net_err_ratelimited("RA: %s got default router without neighbour\n",
+ __func__);
ip6_rt_put(rt);
return;
}
@@ -1136,21 +1132,19 @@ static void ndisc_router_discovery(struct sk_buff *skb)
}
if (rt == NULL && lifetime) {
- ND_PRINTK(3, dbg, "RA: adding default router\n");
+ nd_dbg("adding default router\n");
rt = rt6_add_dflt_router(&ipv6_hdr(skb)->saddr, skb->dev, pref);
if (rt == NULL) {
- ND_PRINTK(0, err,
- "RA: %s failed to add default route\n",
- __func__);
+ net_err_ratelimited("RA: %s failed to add default route\n",
+ __func__);
return;
}
neigh = dst_neigh_lookup(&rt->dst, &ipv6_hdr(skb)->saddr);
if (neigh == NULL) {
- ND_PRINTK(0, err,
- "RA: %s got default router without neighbour\n",
- __func__);
+ net_err_ratelimited("RA: %s got default router without neighbour\n",
+ __func__);
ip6_rt_put(rt);
return;
}
@@ -1218,8 +1212,7 @@ skip_linkparms:
lladdr = ndisc_opt_addr_data(ndopts.nd_opts_src_lladdr,
skb->dev);
if (!lladdr) {
- ND_PRINTK(2, warn,
- "RA: invalid link-layer address length\n");
+ nd_dbg("invalid link-layer address length\n");
goto out;
}
}
@@ -1283,7 +1276,7 @@ skip_routeinfo:
mtu = ntohl(n);
if (mtu < IPV6_MIN_MTU || mtu > skb->dev->mtu) {
- ND_PRINTK(2, warn, "RA: invalid mtu: %d\n", mtu);
+ nd_dbg("invalid mtu: %d\n", mtu);
} else if (in6_dev->cnf.mtu6 != mtu) {
in6_dev->cnf.mtu6 = mtu;
@@ -1304,7 +1297,7 @@ skip_routeinfo:
}
if (ndopts.nd_opts_tgt_lladdr || ndopts.nd_opts_rh) {
- ND_PRINTK(2, warn, "RA: invalid RA options\n");
+ nd_dbg("invalid RA options\n");
}
out:
ip6_rt_put(rt);
@@ -1318,15 +1311,13 @@ static void ndisc_redirect_rcv(struct sk_buff *skb)
switch (skb->ndisc_nodetype) {
case NDISC_NODETYPE_HOST:
case NDISC_NODETYPE_NODEFAULT:
- ND_PRINTK(2, warn,
- "Redirect: from host or unauthorized router\n");
+ nd_dbg("from host or unauthorized router\n");
return;
}
#endif
if (!(ipv6_addr_type(&ipv6_hdr(skb)->saddr) & IPV6_ADDR_LINKLOCAL)) {
- ND_PRINTK(2, warn,
- "Redirect: source address is not link-local\n");
+ nd_dbg("source address is not link-local\n");
return;
}
@@ -1356,15 +1347,13 @@ void ndisc_send_redirect(struct sk_buff *skb, const struct in6_addr *target)
bool ret;
if (ipv6_get_lladdr(dev, &saddr_buf, IFA_F_TENTATIVE)) {
- ND_PRINTK(2, warn, "Redirect: no link-local address on %s\n",
- dev->name);
+ nd_dbg("no link-local address on %s\n", dev->name);
return;
}
if (!ipv6_addr_equal(&ipv6_hdr(skb)->daddr, target) &&
ipv6_addr_type(target) != (IPV6_ADDR_UNICAST|IPV6_ADDR_LINKLOCAL)) {
- ND_PRINTK(2, warn,
- "Redirect: target address is not link-local unicast\n");
+ nd_dbg("target address is not link-local unicast\n");
return;
}
@@ -1383,8 +1372,7 @@ void ndisc_send_redirect(struct sk_buff *skb, const struct in6_addr *target)
rt = (struct rt6_info *) dst;
if (rt->rt6i_flags & RTF_GATEWAY) {
- ND_PRINTK(2, warn,
- "Redirect: destination is not a neighbour\n");
+ nd_dbg("destination is not a neighbour\n");
goto release;
}
peer = inet_getpeer_v6(net->ipv6.peers, &rt->rt6i_dst.addr, 1);
@@ -1397,8 +1385,7 @@ void ndisc_send_redirect(struct sk_buff *skb, const struct in6_addr *target)
if (dev->addr_len) {
struct neighbour *neigh = dst_neigh_lookup(skb_dst(skb), target);
if (!neigh) {
- ND_PRINTK(2, warn,
- "Redirect: no neigh for target address\n");
+ nd_dbg("no neigh for target address\n");
goto release;
}
@@ -1426,9 +1413,8 @@ void ndisc_send_redirect(struct sk_buff *skb, const struct in6_addr *target)
len + hlen + tlen),
1, &err);
if (buff == NULL) {
- ND_PRINTK(0, err,
- "Redirect: %s failed to allocate an skb, err=%d\n",
- __func__, err);
+ net_err_ratelimited("Redirect: %s failed to allocate an skb, err=%d\n",
+ __func__, err);
goto release;
}
@@ -1513,14 +1499,12 @@ int ndisc_rcv(struct sk_buff *skb)
__skb_push(skb, skb->data - skb_transport_header(skb));
if (ipv6_hdr(skb)->hop_limit != 255) {
- ND_PRINTK(2, warn, "NDISC: invalid hop-limit: %d\n",
- ipv6_hdr(skb)->hop_limit);
+ nd_dbg("invalid hop-limit: %d\n", ipv6_hdr(skb)->hop_limit);
return 0;
}
if (msg->icmph.icmp6_code != 0) {
- ND_PRINTK(2, warn, "NDISC: invalid ICMPv6 code: %d\n",
- msg->icmph.icmp6_code);
+ nd_dbg("invalid ICMPv6 code: %d\n", msg->icmph.icmp6_code);
return 0;
}
@@ -1648,9 +1632,8 @@ static int __net_init ndisc_net_init(struct net *net)
err = inet_ctl_sock_create(&sk, PF_INET6,
SOCK_RAW, IPPROTO_ICMPV6, net);
if (err < 0) {
- ND_PRINTK(0, err,
- "NDISC: Failed to initialize the control socket (err %d)\n",
- err);
+ net_err_ratelimited("NDISC: Failed to initialize the control socket (err %d)\n",
+ err);
return err;
}
--
1.7.8.112.g3fd21
^ permalink raw reply related
* Re: [PATCH] ndisc: Use more standard logging styles
From: Joe Perches @ 2012-12-14 4:11 UTC (permalink / raw)
To: David Miller; +Cc: djduanjiong, steffen.klassert, netdev
In-Reply-To: <1355458079.29363.4.camel@joe-AO722>
On Thu, 2012-12-13 at 20:07 -0800, Joe Perches wrote:
> The logging style in this file is "baroque".
Sorry, this is whitespace damaged. Please ignore.
I'll resubmit via git send-email as V2.
(evolution, grumble...)
^ permalink raw reply
* [PATCH] ndisc: Use more standard logging styles
From: Joe Perches @ 2012-12-14 4:07 UTC (permalink / raw)
To: David Miller; +Cc: djduanjiong, steffen.klassert, netdev
In-Reply-To: <20121213.145028.509906659453424680.davem@davemloft.net>
The logging style in this file is "baroque".
Convert indirect uses of net_<level>_ratelimited to
simply use net_<level>_ratelimited.
Add a nd_dbg macro and #define ND_DEBUG for the other
generally inactivated logging messages.
Make those inactivated messages emit only at KERN_DEBUG
instead of other levels. Add "%s: " __func__ to all
these nd_dbg macros and remove the embedded function
names and prefixes.
Signed-off-by: Joe Perches <joe@perches.com>
---
net/ipv6/ndisc.c | 139 ++++++++++++++++++++++++------------------------------
1 files changed, 61 insertions(+), 78 deletions(-)
diff --git a/net/ipv6/ndisc.c b/net/ipv6/ndisc.c
index f2a007b..de1c1f2 100644
--- a/net/ipv6/ndisc.c
+++ b/net/ipv6/ndisc.c
@@ -72,14 +72,19 @@
#include <linux/netfilter.h>
#include <linux/netfilter_ipv6.h>
-/* Set to 3 to get tracing... */
-#define ND_DEBUG 1
+/* #define ND_DEBUG */
-#define ND_PRINTK(val, level, fmt, ...) \
+#ifdef ND_DEBUG
+#define nd_dbg(fmt, ...) \
+ net_dbg_ratelimited("%s: " fmt, __func__, ##__VA_ARGS__)
+#else
+#define nd_dbg(fmt, ...) \
do { \
- if (val <= ND_DEBUG) \
- net_##level##_ratelimited(fmt, ##__VA_ARGS__); \
+ if (0) \
+ net_dbg_ratelimited("%s: " fmt, \
+ __func__, ##__VA_ARGS__); \
} while (0)
+#endif
static u32 ndisc_hash(const void *pkey,
const struct net_device *dev,
@@ -220,9 +225,8 @@ struct ndisc_options *ndisc_parse_options(u8 *opt, int opt_len,
case ND_OPT_MTU:
case ND_OPT_REDIRECT_HDR:
if (ndopts->nd_opt_array[nd_opt->nd_opt_type]) {
- ND_PRINTK(2, warn,
- "%s: duplicated ND6 option found: type=%d\n",
- __func__, nd_opt->nd_opt_type);
+ nd_dbg("duplicated ND6 option found: type=%d\n",
+ nd_opt->nd_opt_type);
} else {
ndopts->nd_opt_array[nd_opt->nd_opt_type] = nd_opt;
}
@@ -250,11 +254,9 @@ struct ndisc_options *ndisc_parse_options(u8 *opt, int opt_len,
* to accommodate future extension to the
* protocol.
*/
- ND_PRINTK(2, notice,
- "%s: ignored unsupported option; type=%d, len=%d\n",
- __func__,
- nd_opt->nd_opt_type,
- nd_opt->nd_opt_len);
+ nd_dbg("ignored unsupported option; type=%d, len=%d\n",
+ nd_opt->nd_opt_type,
+ nd_opt->nd_opt_len);
}
}
opt_len -= l;
@@ -399,8 +401,8 @@ static struct sk_buff *ndisc_build_skb(struct net_device *dev,
len + hlen + tlen),
1, &err);
if (!skb) {
- ND_PRINTK(0, err, "ND: %s failed to allocate an skb, err=%d\n",
- __func__, err);
+ net_err_ratelimited("ND: %s failed to allocate an skb, err=%d\n",
+ __func__, err);
return NULL;
}
@@ -629,9 +631,8 @@ static void ndisc_solicit(struct neighbour *neigh, struct sk_buff *skb)
if ((probes -= neigh->parms->ucast_probes) < 0) {
if (!(neigh->nud_state & NUD_VALID)) {
- ND_PRINTK(1, dbg,
- "%s: trying to ucast probe in NUD_INVALID: %pI6\n",
- __func__, target);
+ net_dbg_ratelimited("%s: trying to ucast probe in NUD_INVALID: %pI6\n",
+ __func__, target);
}
ndisc_send_ns(dev, neigh, target, target, saddr);
} else if ((probes -= neigh->parms->app_probes) < 0) {
@@ -677,7 +678,7 @@ static void ndisc_recv_ns(struct sk_buff *skb)
int is_router = -1;
if (ipv6_addr_is_multicast(&msg->target)) {
- ND_PRINTK(2, warn, "NS: multicast target address\n");
+ nd_dbg("multicast target address\n");
return;
}
@@ -690,20 +691,19 @@ static void ndisc_recv_ns(struct sk_buff *skb)
daddr->s6_addr32[1] == htonl(0x00000000) &&
daddr->s6_addr32[2] == htonl(0x00000001) &&
daddr->s6_addr [12] == 0xff )) {
- ND_PRINTK(2, warn, "NS: bad DAD packet (wrong destination)\n");
+ nd_dbg("bad DAD packet (wrong destination)\n");
return;
}
if (!ndisc_parse_options(msg->opt, ndoptlen, &ndopts)) {
- ND_PRINTK(2, warn, "NS: invalid ND options\n");
+ nd_dbg("invalid ND options\n");
return;
}
if (ndopts.nd_opts_src_lladdr) {
lladdr = ndisc_opt_addr_data(ndopts.nd_opts_src_lladdr, dev);
if (!lladdr) {
- ND_PRINTK(2, warn,
- "NS: invalid link-layer address length\n");
+ nd_dbg("invalid link-layer address length\n");
return;
}
@@ -713,8 +713,7 @@ static void ndisc_recv_ns(struct sk_buff *skb)
* in the message.
*/
if (dad) {
- ND_PRINTK(2, warn,
- "NS: bad DAD packet (link-layer address option)\n");
+ nd_dbg("bad DAD packet (link-layer address option)\n");
return;
}
}
@@ -832,30 +831,29 @@ static void ndisc_recv_na(struct sk_buff *skb)
struct neighbour *neigh;
if (skb->len < sizeof(struct nd_msg)) {
- ND_PRINTK(2, warn, "NA: packet too short\n");
+ nd_dbg("packet too short\n");
return;
}
if (ipv6_addr_is_multicast(&msg->target)) {
- ND_PRINTK(2, warn, "NA: target address is multicast\n");
+ nd_dbg("target address is multicast\n");
return;
}
if (ipv6_addr_is_multicast(daddr) &&
msg->icmph.icmp6_solicited) {
- ND_PRINTK(2, warn, "NA: solicited NA is multicasted\n");
+ nd_dbg("solicited NA is multicasted\n");
return;
}
if (!ndisc_parse_options(msg->opt, ndoptlen, &ndopts)) {
- ND_PRINTK(2, warn, "NS: invalid ND option\n");
+ nd_dbg("invalid ND option\n");
return;
}
if (ndopts.nd_opts_tgt_lladdr) {
lladdr = ndisc_opt_addr_data(ndopts.nd_opts_tgt_lladdr, dev);
if (!lladdr) {
- ND_PRINTK(2, warn,
- "NA: invalid link-layer address length\n");
+ nd_dbg("invalid link-layer address length\n");
return;
}
}
@@ -876,9 +874,8 @@ static void ndisc_recv_na(struct sk_buff *skb)
unsolicited advertisement.
*/
if (skb->pkt_type != PACKET_LOOPBACK)
- ND_PRINTK(1, warn,
- "NA: someone advertises our address %pI6 on %s!\n",
- &ifp->addr, ifp->idev->dev->name);
+ net_warn_ratelimited("NA: someone advertises our address %pI6 on %s!\n",
+ &ifp->addr, ifp->idev->dev->name);
in6_ifa_put(ifp);
return;
}
@@ -940,7 +937,7 @@ static void ndisc_recv_rs(struct sk_buff *skb)
idev = __in6_dev_get(skb->dev);
if (!idev) {
- ND_PRINTK(1, err, "RS: can't find in6 device\n");
+ net_err_ratelimited("RS: can't find in6 device\n");
return;
}
@@ -957,7 +954,7 @@ static void ndisc_recv_rs(struct sk_buff *skb)
/* Parse ND options */
if (!ndisc_parse_options(rs_msg->opt, ndoptlen, &ndopts)) {
- ND_PRINTK(2, notice, "NS: invalid ND option, ignored\n");
+ nd_dbg("invalid ND option, ignored\n");
goto out;
}
@@ -1043,17 +1040,17 @@ static void ndisc_router_discovery(struct sk_buff *skb)
optlen = (skb->tail - skb->transport_header) - sizeof(struct ra_msg);
if (!(ipv6_addr_type(&ipv6_hdr(skb)->saddr) & IPV6_ADDR_LINKLOCAL)) {
- ND_PRINTK(2, warn, "RA: source address is not link-local\n");
+ nd_dbg("source address is not link-local\n");
return;
}
if (optlen < 0) {
- ND_PRINTK(2, warn, "RA: packet too short\n");
+ nd_dbg("packet too short\n");
return;
}
#ifdef CONFIG_IPV6_NDISC_NODETYPE
if (skb->ndisc_nodetype == NDISC_NODETYPE_HOST) {
- ND_PRINTK(2, warn, "RA: from host or unauthorized router\n");
+ nd_dbg("from host or unauthorized router\n");
return;
}
#endif
@@ -1064,13 +1061,13 @@ static void ndisc_router_discovery(struct sk_buff *skb)
in6_dev = __in6_dev_get(skb->dev);
if (in6_dev == NULL) {
- ND_PRINTK(0, err, "RA: can't find inet6 device for %s\n",
- skb->dev->name);
+ net_err_ratelimited("RA: can't find inet6 device for %s\n",
+ skb->dev->name);
return;
}
if (!ndisc_parse_options(opt, optlen, &ndopts)) {
- ND_PRINTK(2, warn, "RA: invalid ND options\n");
+ nd_dbg("invalid ND options\n");
return;
}
@@ -1123,9 +1120,8 @@ static void ndisc_router_discovery(struct sk_buff *skb)
if (rt) {
neigh = dst_neigh_lookup(&rt->dst, &ipv6_hdr(skb)->saddr);
if (!neigh) {
- ND_PRINTK(0, err,
- "RA: %s got default router without neighbour\n",
- __func__);
+ net_err_ratelimited("RA: %s got default router without neighbour\n",
+ __func__);
ip6_rt_put(rt);
return;
}
@@ -1136,21 +1132,19 @@ static void ndisc_router_discovery(struct sk_buff *skb)
}
if (rt == NULL && lifetime) {
- ND_PRINTK(3, dbg, "RA: adding default router\n");
+ nd_dbg("adding default router\n");
rt = rt6_add_dflt_router(&ipv6_hdr(skb)->saddr, skb->dev, pref);
if (rt == NULL) {
- ND_PRINTK(0, err,
- "RA: %s failed to add default route\n",
- __func__);
+ net_err_ratelimited("RA: %s failed to add default route\n",
+ __func__);
return;
}
neigh = dst_neigh_lookup(&rt->dst, &ipv6_hdr(skb)->saddr);
if (neigh == NULL) {
- ND_PRINTK(0, err,
- "RA: %s got default router without neighbour\n",
- __func__);
+ net_err_ratelimited("RA: %s got default router without neighbour\n",
+ __func__);
ip6_rt_put(rt);
return;
}
@@ -1218,8 +1212,7 @@ skip_linkparms:
lladdr = ndisc_opt_addr_data(ndopts.nd_opts_src_lladdr,
skb->dev);
if (!lladdr) {
- ND_PRINTK(2, warn,
- "RA: invalid link-layer address length\n");
+ nd_dbg("invalid link-layer address length\n");
goto out;
}
}
@@ -1283,7 +1276,7 @@ skip_routeinfo:
mtu = ntohl(n);
if (mtu < IPV6_MIN_MTU || mtu > skb->dev->mtu) {
- ND_PRINTK(2, warn, "RA: invalid mtu: %d\n", mtu);
+ nd_dbg("invalid mtu: %d\n", mtu);
} else if (in6_dev->cnf.mtu6 != mtu) {
in6_dev->cnf.mtu6 = mtu;
@@ -1304,7 +1297,7 @@ skip_routeinfo:
}
if (ndopts.nd_opts_tgt_lladdr || ndopts.nd_opts_rh) {
- ND_PRINTK(2, warn, "RA: invalid RA options\n");
+ nd_dbg("invalid RA options\n");
}
out:
ip6_rt_put(rt);
@@ -1318,15 +1311,13 @@ static void ndisc_redirect_rcv(struct sk_buff *skb)
switch (skb->ndisc_nodetype) {
case NDISC_NODETYPE_HOST:
case NDISC_NODETYPE_NODEFAULT:
- ND_PRINTK(2, warn,
- "Redirect: from host or unauthorized router\n");
+ nd_dbg("from host or unauthorized router\n");
return;
}
#endif
if (!(ipv6_addr_type(&ipv6_hdr(skb)->saddr) & IPV6_ADDR_LINKLOCAL)) {
- ND_PRINTK(2, warn,
- "Redirect: source address is not link-local\n");
+ nd_dbg("source address is not link-local\n");
return;
}
@@ -1356,15 +1347,13 @@ void ndisc_send_redirect(struct sk_buff *skb, const struct in6_addr *target)
bool ret;
if (ipv6_get_lladdr(dev, &saddr_buf, IFA_F_TENTATIVE)) {
- ND_PRINTK(2, warn, "Redirect: no link-local address on %s\n",
- dev->name);
+ nd_dbg("no link-local address on %s\n", dev->name);
return;
}
if (!ipv6_addr_equal(&ipv6_hdr(skb)->daddr, target) &&
ipv6_addr_type(target) != (IPV6_ADDR_UNICAST|IPV6_ADDR_LINKLOCAL)) {
- ND_PRINTK(2, warn,
- "Redirect: target address is not link-local unicast\n");
+ nd_dbg("target address is not link-local unicast\n");
return;
}
@@ -1383,8 +1372,7 @@ void ndisc_send_redirect(struct sk_buff *skb, const struct in6_addr *target)
rt = (struct rt6_info *) dst;
if (rt->rt6i_flags & RTF_GATEWAY) {
- ND_PRINTK(2, warn,
- "Redirect: destination is not a neighbour\n");
+ nd_dbg("destination is not a neighbour\n");
goto release;
}
peer = inet_getpeer_v6(net->ipv6.peers, &rt->rt6i_dst.addr, 1);
@@ -1397,8 +1385,7 @@ void ndisc_send_redirect(struct sk_buff *skb, const struct in6_addr *target)
if (dev->addr_len) {
struct neighbour *neigh = dst_neigh_lookup(skb_dst(skb), target);
if (!neigh) {
- ND_PRINTK(2, warn,
- "Redirect: no neigh for target address\n");
+ nd_dbg("no neigh for target address\n");
goto release;
}
@@ -1426,9 +1413,8 @@ void ndisc_send_redirect(struct sk_buff *skb, const struct in6_addr *target)
len + hlen + tlen),
1, &err);
if (buff == NULL) {
- ND_PRINTK(0, err,
- "Redirect: %s failed to allocate an skb, err=%d\n",
- __func__, err);
+ net_err_ratelimited("Redirect: %s failed to allocate an skb, err=%d\n",
+ __func__, err);
goto release;
}
@@ -1513,14 +1499,12 @@ int ndisc_rcv(struct sk_buff *skb)
__skb_push(skb, skb->data - skb_transport_header(skb));
if (ipv6_hdr(skb)->hop_limit != 255) {
- ND_PRINTK(2, warn, "NDISC: invalid hop-limit: %d\n",
- ipv6_hdr(skb)->hop_limit);
+ nd_dbg("invalid hop-limit: %d\n", ipv6_hdr(skb)->hop_limit);
return 0;
}
if (msg->icmph.icmp6_code != 0) {
- ND_PRINTK(2, warn, "NDISC: invalid ICMPv6 code: %d\n",
- msg->icmph.icmp6_code);
+ nd_dbg("invalid ICMPv6 code: %d\n", msg->icmph.icmp6_code);
return 0;
}
@@ -1648,9 +1632,8 @@ static int __net_init ndisc_net_init(struct net *net)
err = inet_ctl_sock_create(&sk, PF_INET6,
SOCK_RAW, IPPROTO_ICMPV6, net);
if (err < 0) {
- ND_PRINTK(0, err,
- "NDISC: Failed to initialize the control socket (err %d)\n",
- err);
+ net_err_ratelimited("NDISC: Failed to initialize the control socket (err %d)\n",
+ err);
return err;
}
^ permalink raw reply related
* Re: [RFC][PATCH] bgmac: driver for GBit MAC core on BCMA bus
From: Joe Perches @ 2012-12-14 1:21 UTC (permalink / raw)
To: Rafał Miłecki; +Cc: netdev, David S. Miller
In-Reply-To: <CACna6rwJZpjdyWjkdoLrcyo3+Pq8CCk4x1N5GS+n8P5prBVm2A@mail.gmail.com>
On Thu, 2012-12-13 at 20:24 +0100, Rafał Miłecki wrote:
> 2012/12/13 Joe Perches <joe@perches.com>:
> > On Thu, 2012-12-13 at 18:43 +0100, Rafał Miłecki wrote:
> >> BCMA is a Broadcom specific bus with devices AKA cores. All recent BCMA
> >> based SoCs have gigabit ethernet provided by the GBit MAC core. This
> >> patch adds driver for such a cores registering itself as a netdev. It
> >> has been tested on a BCM4706 and BCM4718 chipsets.
[]
> >> + /* Stats */
> >> + bool stats_grabbed;
> >> + u32 mib_tx_regs[BGMAC_NUM_MIB_TX_REGS];
> >> + u32 mib_rx_regs[BGMAC_NUM_MIB_RX_REGS];
> >> +};
> >
> > Maybe some minor reordering of the u8/bools to reduce padding.
>
> Really? Compiler won't do that for me? I hope that compiler optimizes
> structs (until using __packed) :(
The compiler is not allowed to reorder struct members.
Perhaps you bundle together all the u8/bools.
Maybe not when/where cachelines are crossed.
^ permalink raw reply
* Re: [PATCH] bridge: Bug fix for incorrect interpretation of MLDv2 maximum response code
From: Stephen Hemminger @ 2012-12-14 0:51 UTC (permalink / raw)
To: Ang Way Chuang; +Cc: netdev, bridge
In-Reply-To: <50CA7449.8000305@sfc.wide.ad.jp>
On Fri, 14 Dec 2012 08:35:21 +0800
Ang Way Chuang <wcang@sfc.wide.ad.jp> wrote:
> On 14/12/2012 08:23, Stephen Hemminger wrote:
> > On Fri, 14 Dec 2012 08:19:04 +0800
> > Ang Way Chuang <wcang@sfc.wide.ad.jp> wrote:
> >
> >> On 14/12/2012 08:12, Stephen Hemminger wrote:
> >>> On Fri, 14 Dec 2012 08:07:01 +0800
> >>> Ang Way Chuang <wcang@sfc.wide.ad.jp> wrote:
> >>>
> >>>> This patch fixes the incorrect interpretation of endianness of MLDv2 maximum response
> >>>> code within bridge's multicast snooping code.
> >>>>
> >>>> Signed-off-by: Ang Way Chuang <wcang@sfc.wide.ad.jp>
> >>>> ---
> >>>> diff --git a/net/bridge/br_multicast.c b/net/bridge/br_multicast.c
> >>>> index 977c3ee..29c6283 100644
> >>>> --- a/net/bridge/br_multicast.c
> >>>> +++ b/net/bridge/br_multicast.c
> >>>> @@ -1172,7 +1172,7 @@ static int br_ip6_multicast_query(struct net_bridge *br,
> >>>> mld2q = (struct mld2_query *)icmp6_hdr(skb);
> >>>> if (!mld2q->mld2q_nsrcs)
> >>>> group = &mld2q->mld2q_mca;
> >>>> - max_delay = mld2q->mld2q_mrc ? MLDV2_MRC(mld2q->mld2q_mrc) : 1;
> >>>> + max_delay = mld2q->mld2q_mrc ? MLDV2_MRC(ntohs(mld2q->mld2q_mrc)) : 1;
> >>>> }
> >>>>
> >>>> if (!group)
> >>> Already fixed as part of my patch to fix sparse warnings.
> >>> --
> >>> To unsubscribe from this list: send the line "unsubscribe netdev" in
> >>> the body of a message to majordomo@vger.kernel.org
> >>> More majordomo info at http://vger.kernel.org/majordomo-info.html
> >>>
> >> aye, aye.
> > Yours looks cleaner (I added a temp variable). You might want to submit
> > patch to switch to your oneline version.
> >
> Well, the patch did get into the netdev mailing list. I think your direct reply arrived in my mailbox first before I received my own patch through netdev. By the way, I don't see your patch on net-next git. Otherwise, I wouldn't have gone through the trouble of debugging and testing it on my computer. Is there any other repo that I should watch out to avoid this kind of silly situation in the future?
You didn't see it because net-next tree is currently closed/idle until Dave opens it up for next release (3.9)
At this time, the only patches are those in the net tree:
git://git.kernel.org/pub/scm/linux/kernel/git/davem/net.git
Which is where fixes for the current release are staged for Linus.
commit eca2a43bb0d2c6ebd528be6acb30a88435abe307
Author: stephen hemminger <shemminger@vyatta.com>
Date: Thu Dec 13 06:51:28 2012 +0000
bridge: fix icmpv6 endian bug and other sparse warnings
Fix the warnings reported by sparse on recent bridge multicast
changes. Mostly just rcu annotation issues but in this case
sparse found a real bug! The ICMPv6 mld2 query mrc
values is in network byte order.
Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
^ permalink raw reply
* Re: [PATCH] bridge: Bug fix for incorrect interpretation of MLDv2 maximum response code
From: Ang Way Chuang @ 2012-12-14 0:35 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: netdev, bridge
In-Reply-To: <20121213162346.71c3779f@nehalam.linuxnetplumber.net>
On 14/12/2012 08:23, Stephen Hemminger wrote:
> On Fri, 14 Dec 2012 08:19:04 +0800
> Ang Way Chuang <wcang@sfc.wide.ad.jp> wrote:
>
>> On 14/12/2012 08:12, Stephen Hemminger wrote:
>>> On Fri, 14 Dec 2012 08:07:01 +0800
>>> Ang Way Chuang <wcang@sfc.wide.ad.jp> wrote:
>>>
>>>> This patch fixes the incorrect interpretation of endianness of MLDv2 maximum response
>>>> code within bridge's multicast snooping code.
>>>>
>>>> Signed-off-by: Ang Way Chuang <wcang@sfc.wide.ad.jp>
>>>> ---
>>>> diff --git a/net/bridge/br_multicast.c b/net/bridge/br_multicast.c
>>>> index 977c3ee..29c6283 100644
>>>> --- a/net/bridge/br_multicast.c
>>>> +++ b/net/bridge/br_multicast.c
>>>> @@ -1172,7 +1172,7 @@ static int br_ip6_multicast_query(struct net_bridge *br,
>>>> mld2q = (struct mld2_query *)icmp6_hdr(skb);
>>>> if (!mld2q->mld2q_nsrcs)
>>>> group = &mld2q->mld2q_mca;
>>>> - max_delay = mld2q->mld2q_mrc ? MLDV2_MRC(mld2q->mld2q_mrc) : 1;
>>>> + max_delay = mld2q->mld2q_mrc ? MLDV2_MRC(ntohs(mld2q->mld2q_mrc)) : 1;
>>>> }
>>>>
>>>> if (!group)
>>> Already fixed as part of my patch to fix sparse warnings.
>>> --
>>> To unsubscribe from this list: send the line "unsubscribe netdev" in
>>> the body of a message to majordomo@vger.kernel.org
>>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>>>
>> aye, aye.
> Yours looks cleaner (I added a temp variable). You might want to submit
> patch to switch to your oneline version.
>
Well, the patch did get into the netdev mailing list. I think your direct reply arrived in my mailbox first before I received my own patch through netdev. By the way, I don't see your patch on net-next git. Otherwise, I wouldn't have gone through the trouble of debugging and testing it on my computer. Is there any other repo that I should watch out to avoid this kind of silly situation in the future?
^ permalink raw reply
* Re: [PATCH] bridge: Bug fix for incorrect interpretation of MLDv2 maximum response code
From: Stephen Hemminger @ 2012-12-14 0:23 UTC (permalink / raw)
To: Ang Way Chuang; +Cc: netdev, bridge
In-Reply-To: <50CA7078.8070204@sfc.wide.ad.jp>
On Fri, 14 Dec 2012 08:19:04 +0800
Ang Way Chuang <wcang@sfc.wide.ad.jp> wrote:
> On 14/12/2012 08:12, Stephen Hemminger wrote:
> > On Fri, 14 Dec 2012 08:07:01 +0800
> > Ang Way Chuang <wcang@sfc.wide.ad.jp> wrote:
> >
> >> This patch fixes the incorrect interpretation of endianness of MLDv2 maximum response
> >> code within bridge's multicast snooping code.
> >>
> >> Signed-off-by: Ang Way Chuang <wcang@sfc.wide.ad.jp>
> >> ---
> >> diff --git a/net/bridge/br_multicast.c b/net/bridge/br_multicast.c
> >> index 977c3ee..29c6283 100644
> >> --- a/net/bridge/br_multicast.c
> >> +++ b/net/bridge/br_multicast.c
> >> @@ -1172,7 +1172,7 @@ static int br_ip6_multicast_query(struct net_bridge *br,
> >> mld2q = (struct mld2_query *)icmp6_hdr(skb);
> >> if (!mld2q->mld2q_nsrcs)
> >> group = &mld2q->mld2q_mca;
> >> - max_delay = mld2q->mld2q_mrc ? MLDV2_MRC(mld2q->mld2q_mrc) : 1;
> >> + max_delay = mld2q->mld2q_mrc ? MLDV2_MRC(ntohs(mld2q->mld2q_mrc)) : 1;
> >> }
> >>
> >> if (!group)
> > Already fixed as part of my patch to fix sparse warnings.
> > --
> > To unsubscribe from this list: send the line "unsubscribe netdev" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at http://vger.kernel.org/majordomo-info.html
> >
> aye, aye.
Yours looks cleaner (I added a temp variable). You might want to submit
patch to switch to your oneline version.
^ permalink raw reply
* Re: [PATCH] bridge: Bug fix for incorrect interpretation of MLDv2 maximum response code
From: Ang Way Chuang @ 2012-12-14 0:19 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: netdev, bridge
In-Reply-To: <20121213161201.6d2da433@nehalam.linuxnetplumber.net>
On 14/12/2012 08:12, Stephen Hemminger wrote:
> On Fri, 14 Dec 2012 08:07:01 +0800
> Ang Way Chuang <wcang@sfc.wide.ad.jp> wrote:
>
>> This patch fixes the incorrect interpretation of endianness of MLDv2 maximum response
>> code within bridge's multicast snooping code.
>>
>> Signed-off-by: Ang Way Chuang <wcang@sfc.wide.ad.jp>
>> ---
>> diff --git a/net/bridge/br_multicast.c b/net/bridge/br_multicast.c
>> index 977c3ee..29c6283 100644
>> --- a/net/bridge/br_multicast.c
>> +++ b/net/bridge/br_multicast.c
>> @@ -1172,7 +1172,7 @@ static int br_ip6_multicast_query(struct net_bridge *br,
>> mld2q = (struct mld2_query *)icmp6_hdr(skb);
>> if (!mld2q->mld2q_nsrcs)
>> group = &mld2q->mld2q_mca;
>> - max_delay = mld2q->mld2q_mrc ? MLDV2_MRC(mld2q->mld2q_mrc) : 1;
>> + max_delay = mld2q->mld2q_mrc ? MLDV2_MRC(ntohs(mld2q->mld2q_mrc)) : 1;
>> }
>>
>> if (!group)
> Already fixed as part of my patch to fix sparse warnings.
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
aye, aye.
^ permalink raw reply
* [PATCH] bridge: Bug fix for incorrect interpretation of MLDv2 maximum response code
From: Ang Way Chuang @ 2012-12-14 0:07 UTC (permalink / raw)
To: netdev; +Cc: Stephen Hemminger, bridge
This patch fixes the incorrect interpretation of endianness of MLDv2 maximum response
code within bridge's multicast snooping code.
Signed-off-by: Ang Way Chuang <wcang@sfc.wide.ad.jp>
---
diff --git a/net/bridge/br_multicast.c b/net/bridge/br_multicast.c
index 977c3ee..29c6283 100644
--- a/net/bridge/br_multicast.c
+++ b/net/bridge/br_multicast.c
@@ -1172,7 +1172,7 @@ static int br_ip6_multicast_query(struct net_bridge *br,
mld2q = (struct mld2_query *)icmp6_hdr(skb);
if (!mld2q->mld2q_nsrcs)
group = &mld2q->mld2q_mca;
- max_delay = mld2q->mld2q_mrc ? MLDV2_MRC(mld2q->mld2q_mrc) : 1;
+ max_delay = mld2q->mld2q_mrc ? MLDV2_MRC(ntohs(mld2q->mld2q_mrc)) : 1;
}
if (!group)
^ permalink raw reply related
* Re: [PATCH] bridge: Bug fix for incorrect interpretation of MLDv2 maximum response code
From: Stephen Hemminger @ 2012-12-14 0:12 UTC (permalink / raw)
To: Ang Way Chuang; +Cc: netdev, bridge
In-Reply-To: <50CA6DA5.1020101@sfc.wide.ad.jp>
On Fri, 14 Dec 2012 08:07:01 +0800
Ang Way Chuang <wcang@sfc.wide.ad.jp> wrote:
> This patch fixes the incorrect interpretation of endianness of MLDv2 maximum response
> code within bridge's multicast snooping code.
>
> Signed-off-by: Ang Way Chuang <wcang@sfc.wide.ad.jp>
> ---
> diff --git a/net/bridge/br_multicast.c b/net/bridge/br_multicast.c
> index 977c3ee..29c6283 100644
> --- a/net/bridge/br_multicast.c
> +++ b/net/bridge/br_multicast.c
> @@ -1172,7 +1172,7 @@ static int br_ip6_multicast_query(struct net_bridge *br,
> mld2q = (struct mld2_query *)icmp6_hdr(skb);
> if (!mld2q->mld2q_nsrcs)
> group = &mld2q->mld2q_mca;
> - max_delay = mld2q->mld2q_mrc ? MLDV2_MRC(mld2q->mld2q_mrc) : 1;
> + max_delay = mld2q->mld2q_mrc ? MLDV2_MRC(ntohs(mld2q->mld2q_mrc)) : 1;
> }
>
> if (!group)
Already fixed as part of my patch to fix sparse warnings.
^ permalink raw reply
* Re: [PATCH] Fix: kmemleak in tcp_v4/6_syn_recv_sock and dccp_v4/6_request_recv_sock
From: Eric Dumazet @ 2012-12-13 23:38 UTC (permalink / raw)
To: Christoph Paasch
Cc: David Miller, Gerrit Renker, Alexey Kuznetsov, James Morris,
Hideaki YOSHIFUJI, Patrick McHardy, netdev, dccp
In-Reply-To: <1355435363-12766-1-git-send-email-christoph.paasch@uclouvain.be>
On Thu, 2012-12-13 at 22:49 +0100, Christoph Paasch wrote:
> If in either of the above functions inet_csk_route_child_sock() or
> __inet_inherit_port() fails, the newsk will not be freed:
>
> unreferenced object 0xffff88022e8a92c0 (size 1592):
> comm "softirq", pid 0, jiffies 4294946244 (age 726.160s)
> hex dump (first 32 bytes):
> 0a 01 01 01 0a 01 01 02 00 00 00 00 a7 cc 16 00 ................
> 02 00 03 01 00 00 00 00 00 00 00 00 00 00 00 00 ................
> backtrace:
> [<ffffffff8153d190>] kmemleak_alloc+0x21/0x3e
> [<ffffffff810ab3e7>] kmem_cache_alloc+0xb5/0xc5
> [<ffffffff8149b65b>] sk_prot_alloc.isra.53+0x2b/0xcd
> [<ffffffff8149b784>] sk_clone_lock+0x16/0x21e
> [<ffffffff814d711a>] inet_csk_clone_lock+0x10/0x7b
> [<ffffffff814ebbc3>] tcp_create_openreq_child+0x21/0x481
> [<ffffffff814e8fa5>] tcp_v4_syn_recv_sock+0x3a/0x23b
> [<ffffffff814ec5ba>] tcp_check_req+0x29f/0x416
> [<ffffffff814e8e10>] tcp_v4_do_rcv+0x161/0x2bc
> [<ffffffff814eb917>] tcp_v4_rcv+0x6c9/0x701
> [<ffffffff814cea9f>] ip_local_deliver_finish+0x70/0xc4
> [<ffffffff814cec20>] ip_local_deliver+0x4e/0x7f
> [<ffffffff814ce9f8>] ip_rcv_finish+0x1fc/0x233
> [<ffffffff814cee68>] ip_rcv+0x217/0x267
> [<ffffffff814a7bbe>] __netif_receive_skb+0x49e/0x553
> [<ffffffff814a7cc3>] netif_receive_skb+0x50/0x82
>
> This happens, because sk_clone_lock initializes sk_refcnt to 2, and thus
> a single sock_put() is not enough to free the memory. Additionally, things
> like xfrm, memcg, cookie_values,... may have been initialized.
> We have to free them properly.
>
> This is fixed by forcing a call to tcp_done(), ending up in
> inet_csk_destroy_sock, doing the final sock_put(). tcp_done() is necessary,
> because it ends up doing all the cleanup on xfrm, memcg, cookie_values,
> xfrm,...
>
> Before calling tcp_done, we have to set the socket to SOCK_DEAD, to
> force it entering inet_csk_destroy_sock. To avoid the warning in
> inet_csk_destroy_sock, inet_num has to be set to 0.
> As inet_csk_destroy_sock does a dec on orphan_count, we first have to
> increase it.
>
> Calling tcp_done() allows us to remove the calls to
> tcp_clear_xmit_timer() and tcp_cleanup_congestion_control().
>
> A similar approach is taken for dccp by calling dccp_done().
>
> This probably should be backported to kernels >= 3.0 as it is in the
> kernel since 0e734419923bd (ipv4: Use inet_csk_route_child_sock() in
> DCCP and TCP.).
Are you sure the above commit is the bug origin ?
It looks like bug was bring by transparent proxy in 2.6.37
commit 093d282321daeb19c107e5f1f16d7f68484f3ade
Author: Balazs Scheidler <bazsi@balabit.hu>
Date: Thu Oct 21 13:06:43 2010 +0200
tproxy: fix hash locking issue when using port redirection in __inet_inherit_port()
When __inet_inherit_port() is called on a tproxy connection the wrong locks are
held for the inet_bind_bucket it is added to. __inet_inherit_port() made an
implicit assumption that the listener's port number (and thus its bind bucket).
Unfortunately, if you're using the TPROXY target to redirect skbs to a
transparent proxy that assumption is not true anymore and things break.
This patch adds code to __inet_inherit_port() so that it can handle this case
by looking up or creating a new bind bucket for the child socket and updates
callers of __inet_inherit_port() to gracefully handle __inet_inherit_port()
failing.
Reported by and original patch from Stephen Buck <stephen.buck@exinda.com>.
See http://marc.info/?t=128169268200001&r=1&w=2 for the original discussion.
Signed-off-by: KOVACS Krisztian <hidden@balabit.hu>
Signed-off-by: Patrick McHardy <kaber@trash.net>
^ permalink raw reply
* Re: [RFT PATCH] 8139cp: properly support change of MTU values [v2]
From: Francois Romieu @ 2012-12-13 22:36 UTC (permalink / raw)
To: John Greene; +Cc: David Miller, netdev, dwmw2
In-Reply-To: <50CA32F9.4090807@redhat.com>
John Greene <jogreene@redhat.com> :
[...]
> Have incorporated this and test it on virtual hardware with on top of
> cb64edb6b89491edfdbae52ba7db9a8b8391d339 (my original work now
> upstream). I plan to submit your work herein upstream as well, with
> attribution to you, if that's ok?
Sure, go ahead.
I have a nasty 8110s (old netgear) bug to keep me busy.
--
Ueimor
^ permalink raw reply
* Re: [PATCH 00/11] Add basic VLAN support to bridges
From: Jamal Hadi Salim @ 2012-12-13 22:56 UTC (permalink / raw)
To: Stephen Hemminger
Cc: David Miller, vyasevic, or.gerlitz, netdev, mst, john.r.fastabend
In-Reply-To: <20121213143712.6932cb22@nehalam.linuxnetplumber.net>
On 12-12-13 05:37 PM, Stephen Hemminger wrote:
>
> You can, run any action before it hits the bridge.
I think you and I have had this discussion before ;->
It works just fine on ingress.
#Add ingress qdisc on br0
tc qdisc add dev br0 ingress
#Add a filter to accept all and count
tc filter add dev br0 parent ffff: protocol ip prio 6 u32 match ip dst
0/0 flowid 1:16 action ok
#show the stats
root@jhs12:~# tc -s filter show parent ffff: dev br0
filter protocol ip pref 6 u32
filter protocol ip pref 6 u32 fh 800: ht divisor 1
filter protocol ip pref 6 u32 fh 800::800 order 2048 key ht 800 bkt 0
flowid 1:16
match 00000000/00000000 at 16
action order 1: gact action pass
random type none pass val 0
index 2 ref 1 bind 1 installed 269 sec used 74 sec
Action statistics:
Sent 1210 bytes 15 pkt (dropped 0, overlimits 0 requeues 0)
backlog 0b 0p requeues 0
------
Look at those packets ...
cheers,
jamal
cheers,
jamal
^ permalink raw reply
* Re: [PATCH 00/11] Add basic VLAN support to bridges
From: Stephen Hemminger @ 2012-12-13 22:37 UTC (permalink / raw)
To: Jamal Hadi Salim
Cc: David Miller, vyasevic, or.gerlitz, netdev, mst, john.r.fastabend
In-Reply-To: <50CA588F.9040800@mojatatu.com>
On Thu, 13 Dec 2012 17:37:03 -0500
Jamal Hadi Salim <jhs@mojatatu.com> wrote:
> On 12-12-13 05:02 PM, Stephen Hemminger wrote:
> > On Thu, 13 Dec 2012 15:17:30 -0500
>
> >
> > Nah, no reinvention, people who want to do filtering and bridging
> > just use openvswitch instead.
>
>
> so says the maintainer of bridging and iproute2.
> Why cant i use bridging as is and iproute2/tc filter action?
> What is the impediment for you my friend? command line cryptic,
> performance problem - what is it?
>
> cheers,
> jamal
>
>
You can, run any action before it hits the bridge.
^ permalink raw reply
* Re: [PATCH 00/11] Add basic VLAN support to bridges
From: Jamal Hadi Salim @ 2012-12-13 22:37 UTC (permalink / raw)
To: Stephen Hemminger
Cc: David Miller, vyasevic, or.gerlitz, netdev, mst, john.r.fastabend
In-Reply-To: <20121213140218.2c705bb5@nehalam.linuxnetplumber.net>
On 12-12-13 05:02 PM, Stephen Hemminger wrote:
> On Thu, 13 Dec 2012 15:17:30 -0500
>
> Nah, no reinvention, people who want to do filtering and bridging
> just use openvswitch instead.
so says the maintainer of bridging and iproute2.
Why cant i use bridging as is and iproute2/tc filter action?
What is the impediment for you my friend? command line cryptic,
performance problem - what is it?
cheers,
jamal
^ permalink raw reply
* Re: netconsole fun
From: Peter Hurley @ 2012-12-13 22:24 UTC (permalink / raw)
To: Neil Horman; +Cc: Cong Wang, netdev
In-Reply-To: <20121213211754.GC14796@hmsreliant.think-freely.org>
On Thu, 2012-12-13 at 16:17 -0500, Neil Horman wrote:
> On Thu, Dec 13, 2012 at 02:27:01PM -0500, Peter Hurley wrote:
> > On Thu, 2012-12-13 at 13:08 -0500, Neil Horman wrote:
> > > On Thu, Dec 13, 2012 at 09:49:31AM -0500, Peter Hurley wrote:
> > > > On Thu, 2012-12-13 at 07:36 -0500, Neil Horman wrote:
> > > > > On Wed, Dec 12, 2012 at 03:59:17PM -0500, Peter Hurley wrote:
> > > > > > On Tue, 2012-12-11 at 11:45 -0500, Neil Horman wrote:
> > > > > > > On Tue, Dec 11, 2012 at 10:16:51AM -0500, Peter Hurley wrote:
> > > > > > > > On Tue, 2012-12-11 at 09:30 -0500, Neil Horman wrote:
> > > > > > > > > On Tue, Dec 11, 2012 at 09:19:52AM -0500, Peter Hurley wrote:
> > > > > > > > > > On Tue, 2012-12-11 at 04:51 +0000, Cong Wang wrote:
> > > > > > > > > > > On Mon, 10 Dec 2012 at 14:17 GMT, Peter Hurley <peter@hurleysoftware.com> wrote:
> > > > > > > > > > > > Now that netpoll has been disabled for slaved devices, is there a
> > > > > > > > > > > > recommended method of running netconsole on a machine that has a slaved
> > > > > > > > > > > > device?
> > > > > > > > > > > >
> > > > > > > > > > >
> > > > > > > > > > > Yes, running it on the master device instead.
> > > > > > > > > >
> > > > > > > > > > Thanks for the suggestion, but:
> > > > > > > > > >
> > > > > > > > > > [ 0.000000] Kernel command line: BOOT_IMAGE=/boot/vmlinuz-3.7.0-rc8-xeon ...... netconsole=@192.168.10.99/br0,30000@192.168.10.100/xx:xx:xx:xx:xx:xx
> > > > > > > > > > ...
> > > > > > > > > > [ 5.289869] netpoll: netconsole: local port 6665
> > > > > > > > > > [ 5.289885] netpoll: netconsole: local IP 192.168.10.99
> > > > > > > > > > [ 5.289892] netpoll: netconsole: interface 'br0'
> > > > > > > > > > [ 5.289898] netpoll: netconsole: remote port 30000
> > > > > > > > > > [ 5.289907] netpoll: netconsole: remote IP 192.168.10.100
> > > > > > > > > > [ 5.289914] netpoll: netconsole: remote ethernet address xx:xx:xx:xx:xx:xx
> > > > > > > > > > [ 5.289922] netpoll: netconsole: br0 doesn't exist, aborting
> > > > > > > > > > [ 5.289929] netconsole: cleaning up
> > > > > > > > > > ...
> > > > > > > > > > [ 9.392291] Bridge firewalling registered
> > > > > > > > > > [ 9.396805] device eth1 entered promiscuous mode
> > > > > > > > > > [ 9.418350] eth1: setting full-duplex.
> > > > > > > > > > [ 9.421268] br0: port 1(eth1) entered forwarding state
> > > > > > > > > > [ 9.423354] br0: port 1(eth1) entered forwarding state
> > > > > > > > > >
> > > > > > > > > >
> > > > > > > > > > Is there a way to control or associate network device names prior to
> > > > > > > > > > udev renaming?
> > > > > > > > > >
> > > > > > > > > That looks like a systemd problem (or more specifically a boot dependency
> > > > > > > > > problem). You need to modify your netconsole unit/service file to start after
> > > > > > > > > all your networking is up. NetworkManager provides a dummy service file for
> > > > > > > > > this purpose, called networkmanager-wait-online.service
> > > > > > > >
> > > > > > > > Ok. So with a single physical network interface that will be bridged,
> > > > > > > > netconsole cannot used for kernel boot messages.
> > > > > > > >
> > > > > > > > With a machine with multiple nics, is there a way to control device
> > > > > > > > naming so that the interface name to be used by netconsole specified on
> > > > > > > > the boot command line will actually corresponding to the intended
> > > > > > > > device. For example,
> > > > > > > >
> > > > > > > > [ 0.000000] Kernel command line: BOOT_IMAGE=/boot/vmlinuz-3.7.0-rc8-xeon ...... netconsole=@192.168.1.123/eth0,30000@192.168.1.139/xx:xx:xx:xx:xx:xx
> > > > > > > > ....
> > > > > > > > [ 4.092184] 3c59x: Donald Becker and others.
> > > > > > > > [ 4.092204] 0000:07:05.0: 3Com PCI 3c905C Tornado at ffffc9000186cf80.
> > > > > > > > [ 4.094035] tg3.c:v3.125 (September 26, 2012)
> > > > > > > > ....
> > > > > > > > [ 4.125038] tg3 0000:08:00.0 eth1: Tigon3 [partno(BCM95754) rev b002] (PCI Express) MAC address xx:xx:xx:xx:xx:xx
> > > > > > > > [ 4.125055] tg3 0000:08:00.0 eth1: attached PHY is 5787 (10/100/1000Base-T Ethernet) (WireSpeed[1], EEE[0])
> > > > > > > > [ 4.125062] tg3 0000:08:00.0 eth1: RXcsums[1] LinkChgREG[0] MIirq[0] ASF[0] TSOcap[1]
> > > > > > > > [ 4.125068] tg3 0000:08:00.0 eth1: dma_rwctrl[76180000] dma_mask[64-bit]
> > > > > > > >
> > > > > > > > This is attaching netconsole to the wrong device because bus
> > > > > > > > enumeration, and therefore load order, is not consistent from boot to
> > > > > > > > boot.
> > > > > > > >
> > > > > > > No, theres no way to do that. As you note device ennumeration isn't consistent
> > > > > > > accross boots, thats why udev creates rules to rename devices based on immutable
> > > > > > > (or semi-immutable) data, like mac addresses, or pci bus locations). Once that
> > > > > > > happens, you'll have consistent names for your interfaces, and that work will be
> > > > > > > guaranteed to be done after networkmanager has finished opening all the
> > > > > > > interfaces that it needs (hence my suggestion to make netconsole service
> > > > > > > dependent on networkmanager service startup completing).
> > > > > >
> > > > > > Just wondering if you think something like the patch below is
> > > > > > suitable/acceptable for insulating netconsole from inconsistent device
> > > > > > name scenarios without changing the existing semantics. The basic idea
> > > > > > is to allow an ethernet MAC address in the <dev> field of the
> > > > > > netconsole= options, and if a MAC address was specified rather than a
> > > > > > device name, to do the dev lookup from the MAC address instead.
> > > > > >
> > > > > > This doesn't extend to, but also doesn't interfere with, the dynamic
> > > > > > config of netconsole via configfs.
> > > > > >
> > > > > > Would you mind reviewing it?
> > > > > >
> > > > > > Regards,
> > > > > > Peter
> > > > > >
> > > > > This looks like a pretty good idea to me. That said, something occured to me
> > > > > when you wrote your summary above. Have you looked at the netconsole service
> > > > > scripts that most distros provide in their packaging? I'm almost positive Red
> > > > > Hat/Fedora (and also like Suse and Ubuntu), already implement this functionality
> > > > > from user space. Basically, instead of people just modprobing netconsole, they
> > > > > create a service script that parses a config file that has contains all the
> > > > > options needed to load the netconsole module, and it has the intellegence to see
> > > > > if you specified a mac address rather than a device. If you did that it finds
> > > > > the corresponding device mac address and uses that as the device. I'm sorry, I
> > > > > don't know why I didn't think of that before. Check that out though, that will
> > > > > likey give you exactly what you need
> > > >
> > > > Even with a udev rule to load netconsole that runs immediately after
> > > > device renaming (so before scripting), most of the dynamic module
> > > > loading has already happened so netconsole misses it. At least with the
> > > > patch, netconsole will load and attach to the proper interface much
> > > > earlier in the boot so that module-load-time messages will be caught.
> > > >
> > > I'm not sure what you mean by this.
> >
> > This is the beginning of my netconsole log if I use userspace scripts to
> > start it.
> >
> > [ 19.125314] ip_tables: (C) 2000-2006 Netfilter Core Team
> > [ 20.060925] nf_conntrack version 0.5.0 (16384 buckets, 65536 max)
> > [ 21.829331] ip6_tables: (C) 2000-2006 Netfilter Core Team
> > [ 25.728370] at-spi-registry[1862]: segfault at 18 ip 00007f6dd1dd45f1 sp 00007fff49bcd760 error 4 in libgconf-2.so.4.1.5[7f6dd1dbd000+2d000]
> > [ 26.778848] EXT4-fs (dm-3): re-mounted. Opts: errors=remount-ro,commit=0
> > [ 30.643469] Bluetooth: RFCOMM TTY layer initialized
> > [ 30.643509] Bluetooth: RFCOMM socket layer initialized
> > [ 30.643512] Bluetooth: RFCOMM ver 1.11
> > [ 30.784550] Bluetooth: BNEP (Ethernet Emulation) ver 1.3
> > [ 30.784567] Bluetooth: BNEP filters: protocol multicast
> > [ 30.784584] Bluetooth: BNEP socket layer initialized
> > [ 34.010813] init: plymouth-stop pre-start process (2205) terminated with status 1
> >
> > This is the beginning of my netconsole log if I am able to specify
> > netconsole= options on the boot command line. Netconsole starts logging
> > much earlier because it is much loaded earlier.
> >
> > [ 8.764336] EXT4-fs (dm-4): mounted filesystem with ordered data mode. Opts: (null)
> > [ 9.409379] firewire_core 0000:07:06.0: created device fw1: GUID 0800460301c2d69e, S400
> > [ 9.567395] init: ureadahead main process (500) terminated with status 5
> > [ 10.400338] Adding 10996456k swap on /dev/mapper/isw_cbdbfhdjad_Raid0p5. Priority:-1 extents:1 across:10996456k
> > [ 10.496974] udevd[541]: starting version 173
> > [ 10.725906] EXT4-fs (dm-4): re-mounted. Opts: errors=remount-ro
> > [ 11.288352] lp: driver loaded but no devices found
> > [ 12.240058] parport_pc 00:05: reported by Plug and Play ACPI
> > [ 12.240145] parport0: PC-style at 0x378 (0x778), irq 7, using FIFO [PCSPP,TRISTATE,COMPAT,ECP]
> > [ 12.336161] lp0: using parport0 (interrupt-driven).
> > [ 12.342867] microcode: CPU0 sig=0x10676, pf=0x40, revision=0x60f
> > [ 12.436657] shpchp: Standard Hot Plug PCI Controller Driver version: 0.4
> > [ 12.442245] ppdev: user-space parallel port driver
> > [ 12.451592] net firewire0: IPv4 over IEEE 1394 on card 0000:07:06.0
> >
> > Does that make more sense now?
> >
> No, actually, what exactly are you trying to show me here? I don't see any
> indication of netconsole doing anything in either of these log snippets.
^^^^^^^^^^^^^^^^^^^^^^
except that it was netconsole that wrote these logs
Note the log times.
In the first log, which is from a netconsole loaded by userspace
scripts, the first printk it logs isn't until 19.125314 secs into boot.
Most kernel + module init has already happened by this time.
In the second log, which is from a netconsole (still built as a module)
loaded as a result of using the boot command line. It starts logging at
8.764336 secs into boot -- almost 10 secs earlier than using userspace
scripting to load netconsole.
> I'm
> also not sure why you're specifying netconsole options on the kernel command
> line at all.
> Can you elaborate?
Specifying netconsole= on the boot command line loads the netconsole
module at the earliest possible time (and much earlier than scripting
will do). And also leaves it as an optional configuration (at least for
me it does since I use grub2 and can edit the boot command line before
booting).
AFAIK, there are 5 ways to load netconsole:
1. On the boot command line with netconsole=
If netconsole is built-in, this is the only way to initialize it.
If netconsole is a module, this forces netconsole to load at the
earliest possible time.
2. Via /etc/modules
This happens before network device renaming, so suffers from the
problems we've been discussing.
3. Via a custom udev rule in /etc/udev/rules.d/
Earliest userspace method to modprobe netconsole and can be used
after device renaming, but is still fairly late in the boot process.
4. Via init/service scripting
5. At the user shell
AFAIK, there are 4 ways to specify the necessary options to netconsole:
a. On the boot command line with netconsole=
Also loads netconsole
b. In a .conf file in /etc/modprobe.d
c. On the modprobe command line
d. Via configfs
Regards,
Peter
^ permalink raw reply
* Re: [tcpdump-workers] vlan tagged packets and libpcap breakage
From: Ani Sinha @ 2012-12-13 22:07 UTC (permalink / raw)
To: Daniel Borkmann
Cc: Michael Richardson, netdev, tcpdump-workers, Francesco Ruggeri
In-Reply-To: <CAD6jFUTht82HOjGjDU7hFCEWyE3TOx_W4_j=SZK-DrcGfrio-A@mail.gmail.com>
On Thu, Dec 13, 2012 at 1:49 PM, Daniel Borkmann
<danborkmann@iogearbox.net> wrote:
> Well, not everyone on netdev might be following this thread (resp.
> following fully). The best way to get responses for a patch is to go
> through the normal patch submission process on netdev, and if you like
> to request for comments, then mark it as RFC in the subject. This way,
> people will know and likely comment on it if it makes sense or not.
OK good to know.
> As Eric mentioned earlier, for now there seems not to be a reliable
> way to get to know which ops are present and which not. It's not
> really nice, but if you want to make use of those new (ANC*) features,
> probably checking kernel version might be the only way if I'm not
> missing something. Now net-next is closed, but if it reopens, I'll
> submit a version 2 of my patch where you've been CC'd to. If it gets
> in, then at least it's for sure that since kernel <xyz> this kind of
> feature test is present.
thanks, yes, I believe we do need some sort of validation on the
ancilliary features.
^ permalink raw reply
* Re: [PATCH 00/11] Add basic VLAN support to bridges
From: Stephen Hemminger @ 2012-12-13 22:02 UTC (permalink / raw)
To: Jamal Hadi Salim
Cc: David Miller, vyasevic, or.gerlitz, netdev, mst, john.r.fastabend
In-Reply-To: <50CA37DA.1090905@mojatatu.com>
On Thu, 13 Dec 2012 15:17:30 -0500
Jamal Hadi Salim <jhs@mojatatu.com> wrote:
> On 12-12-13 02:04 PM, Stephen Hemminger wrote:
> >
> > Maybe a better filtering architecture (at the bridge level) would
> > be a good project.
> >
>
> Ok, please before you go off and re-invent the winter tire - take a look
> at tc filtering which works well with any netdev and point out what the
> issues are.
>
>
> cheers,
> jamal
Nah, no reinvention, people who want to do filtering and bridging
just use openvswitch instead.
^ permalink raw reply
* Re: [GIT] Networking
From: Olof Johansson @ 2012-12-13 21:52 UTC (permalink / raw)
To: Linus Torvalds
Cc: David Miller, Andrew Morton, Network Development,
Linux Kernel Mailing List, Arnd Bergmann, linux-arm-kernel,
Tony Lindgren
In-Reply-To: <CA+55aFwzUgxQAze=mYbEx8b61V542tzm06Df=mR1BtYVbJy0mg@mail.gmail.com>
On Thu, Dec 13, 2012 at 3:15 AM, Linus Torvalds
<torvalds@linux-foundation.org> wrote:
> On Wed, Dec 12, 2012 at 12:11 PM, David Miller <davem@davemloft.net> wrote:
>>
>> There is one merge conflict to resolve in net/sched/cls_cgroup.c,
>> one commit changes the name of some members to "css_*" (this came
>> from Tejun's tree) and another commit adds an "attach" method.
>
> There's more than that. The ARM board mess is apparently now affecting
> the networking merges too.
>
> I fixed it up. Hopefully correctly.
Yes, it looks correct. Sorry about missing that earlier, sfr had
spotted it when it showed up in linux-next but I lost track of it.
We probably need to restructure/resort the device tree files to avoid
add/add conflicts when people just append to them all the time. I
really hope things will start to settle down churn-wise soon, as
conversions and DT enablement starts to be more complete.
(I also think we need to improve the way we handle our topics some, we
have a bit too much internal conflicts between topics in arm-soc
still, but that's a separate subject).
-Olof
^ 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