* Re: [PATCH 1/2] [NETFILTER] PPTP helper: Fix compilation of conntrack helper without NAT
From: Harald Welte @ 2005-11-03 13:27 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo, Linux Netdev List,
Netfilter Development Mailinglist
In-Reply-To: <20051103131917.GJ4479@sunbeam.de.gnumonks.org>
[-- Attachment #1: Type: text/plain, Size: 3000 bytes --]
Ok, here goes the real fix for [1/2]:
[NETFILTER] PPTP helper: Fix compilation of conntrack helper without NAT
This patch fixes compilation of the PPTP conntrack helper when NAT is
configured off.
Signed-off-by: Yasuyuki Kozakai <yasuyuki.kozakai@toshiba.co.jp>
Signed-off-by: Harald Welte <laforge@netfilter.org>
---
commit f6447dcd8a4f13b144c9f49df76682c31562a003
tree e5f7c3a472ba896ee793727fbe4de02f42047d75
parent 928f936ad8e099bd2a84d6e875e1a8c59f55bf7d
author Harald Welte <laforge@hanuman.de.gnumonks.org> Thu, 03 Nov 2005 10:56:16 +0100
committer Harald Welte <laforge@hanuman.de.gnumonks.org> Thu, 03 Nov 2005 10:56:16 +0100
net/ipv4/netfilter/ip_conntrack_helper_pptp.c | 4 ----
net/ipv4/netfilter/ip_nat_helper_pptp.c | 2 ++
2 files changed, 2 insertions(+), 4 deletions(-)
diff --git a/net/ipv4/netfilter/ip_conntrack_helper_pptp.c b/net/ipv4/netfilter/ip_conntrack_helper_pptp.c
--- a/net/ipv4/netfilter/ip_conntrack_helper_pptp.c
+++ b/net/ipv4/netfilter/ip_conntrack_helper_pptp.c
@@ -270,14 +270,10 @@ exp_gre(struct ip_conntrack *master,
exp_orig->expectfn = pptp_expectfn;
exp_orig->flags = 0;
- exp_orig->dir = IP_CT_DIR_ORIGINAL;
-
/* both expectations are identical apart from tuple */
memcpy(exp_reply, exp_orig, sizeof(*exp_reply));
memcpy(&exp_reply->tuple, &exp_tuples[1], sizeof(exp_reply->tuple));
- exp_reply->dir = !exp_orig->dir;
-
if (ip_nat_pptp_hook_exp_gre)
ret = ip_nat_pptp_hook_exp_gre(exp_orig, exp_reply);
else {
diff --git a/net/ipv4/netfilter/ip_nat_helper_pptp.c b/net/ipv4/netfilter/ip_nat_helper_pptp.c
--- a/net/ipv4/netfilter/ip_nat_helper_pptp.c
+++ b/net/ipv4/netfilter/ip_nat_helper_pptp.c
@@ -216,6 +216,7 @@ pptp_exp_gre(struct ip_conntrack_expect
expect_orig->saved_proto.gre.key = htons(nat_pptp_info->pac_call_id);
expect_orig->tuple.src.u.gre.key = htons(nat_pptp_info->pns_call_id);
expect_orig->tuple.dst.u.gre.key = htons(ct_pptp_info->pac_call_id);
+ expect_orig->dir = IP_CT_DIR_ORIGINAL;
inv_t.src.ip = reply_t->src.ip;
inv_t.dst.ip = reply_t->dst.ip;
inv_t.src.u.gre.key = htons(nat_pptp_info->pac_call_id);
@@ -233,6 +234,7 @@ pptp_exp_gre(struct ip_conntrack_expect
expect_reply->saved_proto.gre.key = htons(nat_pptp_info->pns_call_id);
expect_reply->tuple.src.u.gre.key = htons(nat_pptp_info->pac_call_id);
expect_reply->tuple.dst.u.gre.key = htons(ct_pptp_info->pns_call_id);
+ expect_reply->dir = IP_CT_DIR_REPLY;
inv_t.src.ip = orig_t->src.ip;
inv_t.dst.ip = orig_t->dst.ip;
inv_t.src.u.gre.key = htons(nat_pptp_info->pns_call_id);
--
- Harald Welte <laforge@netfilter.org> http://netfilter.org/
============================================================================
"Fragmentation is like classful addressing -- an interesting early
architectural error that shows how much experimentation was going
on while IP was being designed." -- Paul Vixie
[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]
^ permalink raw reply
* Re: [PATCH][MCAST]Two fix for implementation of MLDv2 .
From: Yan Zheng @ 2005-11-03 13:26 UTC (permalink / raw)
To: netdev; +Cc: linux-kernel, David Stevens
In-Reply-To: <436878E7.3030303@21cn.com>
You can test change in add_grec(...) by follow codes. without the
change no change to exclude report will be sent.
please adjust IFINDEX and make sure initial state for FF02::2 is idle
===================================================
#include <stdio.h>
#include <unistd.h>
#include <netinet/in.h>
#include <arpa/inet.h>
#include <sys/socket.h>
#include <sys/types.h>
#define IFINDEX 6
int main(int argc, char argv[])
{
int sockfds[2];
struct ipv6_mreq req;
struct group_filter filter;
struct sockaddr_in6 *psin6;
req.ipv6mr_interface = IFINDEX;
inet_pton(PF_INET6, "FF02::2", &req.ipv6mr_multiaddr);
sockfds[0] = socket(PF_INET6, SOCK_DGRAM, 0);
sockfds[1] = socket(PF_INET6, SOCK_DGRAM, 0);
filter.gf_interface = IFINDEX;
filter.gf_fmode = MCAST_INCLUDE;
filter.gf_numsrc = 1;
psin6 = (struct sockaddr_in6 *)&filter.gf_group;
psin6->sin6_family = AF_INET6;
inet_pton(PF_INET6, "FF02::2", &psin6->sin6_addr);
psin6 = (struct sockaddr_in6 *)&filter.gf_slist[0];
psin6->sin6_family = AF_INET6;
inet_pton(PF_INET6, "2002:de12:1780::1", &psin6->sin6_addr);
setsockopt(sockfds[0], SOL_IPV6, IPV6_ADD_MEMBERSHIP, &req, sizeof(req));
setsockopt(sockfds[0], SOL_IPV6, MCAST_MSFILTER, &filter, sizeof(filter));
sleep(5);
setsockopt(sockfds[1], SOL_IPV6, IPV6_ADD_MEMBERSHIP, &req, sizeof(req));
pause();
return 0;
}
^ permalink raw reply
* Re: [PATCH 1/2] [NETFILTER] PPTP helper: Fix compilation of conntrack helper without NAT
From: Harald Welte @ 2005-11-03 13:19 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo, Linux Netdev List,
Netfilter Development Mailinglist
In-Reply-To: <20051103120404.GB4479@sunbeam.de.gnumonks.org>
[-- Attachment #1: Type: text/plain, Size: 707 bytes --]
On Thu, Nov 03, 2005 at 01:04:04PM +0100, Harald Welte wrote:
> Hi Acme!
>
> Please merge this to both 2.6.15 mainline and 2.6.14.x stable series,
> thanks.
*sigh*. this is not my day. Please do _not_ apply this patch number 1.
I'll re-send a replacement.
Patch number '2' is fine and should go in.
Sorry for the mess.
--
- Harald Welte <laforge@netfilter.org> http://netfilter.org/
============================================================================
"Fragmentation is like classful addressing -- an interesting early
architectural error that shows how much experimentation was going
on while IP was being designed." -- Paul Vixie
[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]
^ permalink raw reply
* [PATCH 2/2] [NETFILTER] PPTP helper: Fix endianness bug in GRE key / CallID NAT
From: Harald Welte @ 2005-11-03 12:05 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: Linux Netdev List, Netfilter Development Mailinglist
[-- Attachment #1: Type: text/plain, Size: 1860 bytes --]
Hi Acme!
Please merge this to both 2.6.15 mainline and 2.6.14.x stable series,
thanks.
[NETFILTER] PPTP helper: Fix endianness bug in GRE key / CallID NAT
This endianness bug slipped through while changing the 'gre.key' field in the
conntrack tuple from 32bit to 16bit.
None of my tests caught the problem, since the linux pptp client always has
'0' as call id / gre key. Only windows clients actually trigger the bug.
Signed-off-by: Harald Welte <laforge@netfilter.org>
---
commit 66d73ebf14b8bb18647eabca9975d7ceb787cb29
tree b92bab972b99c11ead4ca05d152ab78f741fc41f
parent f6447dcd8a4f13b144c9f49df76682c31562a003
author Harald Welte <laforge@hanuman.de.gnumonks.org> Thu, 03 Nov 2005 12:55:06 +0100
committer Harald Welte <laforge@hanuman.de.gnumonks.org> Thu, 03 Nov 2005 12:55:06 +0100
net/ipv4/netfilter/ip_nat_proto_gre.c | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/net/ipv4/netfilter/ip_nat_proto_gre.c b/net/ipv4/netfilter/ip_nat_proto_gre.c
--- a/net/ipv4/netfilter/ip_nat_proto_gre.c
+++ b/net/ipv4/netfilter/ip_nat_proto_gre.c
@@ -139,8 +139,8 @@ gre_manip_pkt(struct sk_buff **pskb,
break;
case GRE_VERSION_PPTP:
DEBUGP("call_id -> 0x%04x\n",
- ntohl(tuple->dst.u.gre.key));
- pgreh->call_id = htons(ntohl(tuple->dst.u.gre.key));
+ ntohs(tuple->dst.u.gre.key));
+ pgreh->call_id = tuple->dst.u.gre.key;
break;
default:
DEBUGP("can't nat unknown GRE version\n");
--
- Harald Welte <laforge@netfilter.org> http://netfilter.org/
============================================================================
"Fragmentation is like classful addressing -- an interesting early
architectural error that shows how much experimentation was going
on while IP was being designed." -- Paul Vixie
[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]
^ permalink raw reply
* [PATCH 1/2] [NETFILTER] PPTP helper: Fix compilation of conntrack helper without NAT
From: Harald Welte @ 2005-11-03 12:04 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: Linux Netdev List, Netfilter Development Mailinglist
[-- Attachment #1: Type: text/plain, Size: 3054 bytes --]
Hi Acme!
Please merge this to both 2.6.15 mainline and 2.6.14.x stable series,
thanks.
[NETFILTER] PPTP helper: Fix compilation of conntrack helper without NAT
This patch fixes compilation of the PPTP conntrack helper when NAT is
configured off.
Signed-off-by: Yasuyuki Kozakai <yasuyuki.kozakai@toshiba.co.jp>
Signed-off-by: Harald Welte <laforge@netfilter.org>
---
commit f6447dcd8a4f13b144c9f49df76682c31562a003
tree e5f7c3a472ba896ee793727fbe4de02f42047d75
parent 928f936ad8e099bd2a84d6e875e1a8c59f55bf7d
author Harald Welte <laforge@hanuman.de.gnumonks.org> Thu, 03 Nov 2005 10:56:16 +0100
committer Harald Welte <laforge@hanuman.de.gnumonks.org> Thu, 03 Nov 2005 10:56:16 +0100
net/ipv4/netfilter/ip_conntrack_helper_pptp.c | 4 ----
net/ipv4/netfilter/ip_nat_helper_pptp.c | 2 ++
2 files changed, 2 insertions(+), 4 deletions(-)
diff --git a/net/ipv4/netfilter/ip_conntrack_helper_pptp.c b/net/ipv4/netfilter/ip_conntrack_helper_pptp.c
--- a/net/ipv4/netfilter/ip_conntrack_helper_pptp.c
+++ b/net/ipv4/netfilter/ip_conntrack_helper_pptp.c
@@ -270,14 +270,10 @@ exp_gre(struct ip_conntrack *master,
exp_orig->expectfn = pptp_expectfn;
exp_orig->flags = 0;
- exp_orig->dir = IP_CT_DIR_ORIGINAL;
-
/* both expectations are identical apart from tuple */
memcpy(exp_reply, exp_orig, sizeof(*exp_reply));
memcpy(&exp_reply->tuple, &exp_tuples[1], sizeof(exp_reply->tuple));
- exp_reply->dir = !exp_orig->dir;
-
if (ip_nat_pptp_hook_exp_gre)
ret = ip_nat_pptp_hook_exp_gre(exp_orig, exp_reply);
else {
diff --git a/net/ipv4/netfilter/ip_nat_helper_pptp.c b/net/ipv4/netfilter/ip_nat_helper_pptp.c
--- a/net/ipv4/netfilter/ip_nat_helper_pptp.c
+++ b/net/ipv4/netfilter/ip_nat_helper_pptp.c
@@ -216,6 +216,7 @@ pptp_exp_gre(struct ip_conntrack_expect
expect_orig->saved_proto.gre.key = htons(nat_pptp_info->pac_call_id);
expect_orig->tuple.src.u.gre.key = htons(nat_pptp_info->pns_call_id);
expect_orig->tuple.dst.u.gre.key = htons(ct_pptp_info->pac_call_id);
+ expect_orig->dir = IP_CT_DIR_ORIGINAL;
inv_t.src.ip = reply_t->src.ip;
inv_t.dst.ip = reply_t->dst.ip;
inv_t.src.u.gre.key = htons(nat_pptp_info->pac_call_id);
@@ -233,6 +234,7 @@ pptp_exp_gre(struct ip_conntrack_expect
expect_reply->saved_proto.gre.key = htons(nat_pptp_info->pns_call_id);
expect_reply->tuple.src.u.gre.key = htons(nat_pptp_info->pac_call_id);
expect_reply->tuple.dst.u.gre.key = htons(ct_pptp_info->pns_call_id);
+ expect_reply->dir = !exp_orig->dir;
inv_t.src.ip = orig_t->src.ip;
inv_t.dst.ip = orig_t->dst.ip;
inv_t.src.u.gre.key = htons(nat_pptp_info->pns_call_id);
--
- Harald Welte <laforge@netfilter.org> http://netfilter.org/
============================================================================
"Fragmentation is like classful addressing -- an interesting early
architectural error that shows how much experimentation was going
on while IP was being designed." -- Paul Vixie
[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]
^ permalink raw reply
* Re: [PATCH] Merge __load_pointer() and load_pointer() in net/core/filter.c; kernel 2.6.14
From: Mitchell Blank Jr @ 2005-11-03 6:58 UTC (permalink / raw)
To: Kris Katterjohn; +Cc: Herbert Xu, jschlst, davem, acme, netdev, linux-kernel
In-Reply-To: <5b1bc8f2a7f34523b323fc1b58ef4c26.kjak@ispwest.com>
Kris Katterjohn wrote:
> > From: Mitchell Blank Jr
> > > (I trimmed the cc: list a bit; no need for this to be on LKML in my opinion)
I see you just added it back. Oh well.
> > So I guess use my patch and take "inline" off? What do you think?
Well the original author presumably thought that the fast-path of
load_pointer() was critical enough to keep inline (since it can be run many
times per packet) So they made the deliberate choice of separating it
into two functions - one inline, one non-inline.
So my personal feeling is that the code is probably fine as it stands today.
> Maybe "static" should be removed, too? Oh well.
Uh, why? It's clearly a file-local function.
-Mitch
^ permalink raw reply
* Re: Re: [PATCH] Merge __load_pointer() and load_pointer() in net/core/filter.c; kernel 2.6.14
From: Kris Katterjohn @ 2005-11-03 6:54 UTC (permalink / raw)
To: Mitchell Blank Jr; +Cc: Herbert Xu, jschlst, davem, acme, netdev, linux-kernel
> > > From: Mitchell Blank Jr
> > > > (I trimmed the cc: list a bit; no need for this to be on LKML in my opinion)
>
> I see you just added it back. Oh well.
When I emailed (CC:) Herbert Xu (who emailed about this earlier), I got an email
back that said it couldn't send to him. So, I just added linux-kernel back so
he'd see it if he's interested at all.
> > > So I guess use my patch and take "inline" off? What do you think?
>
> Well the original author presumably thought that the fast-path of
> load_pointer() was critical enough to keep inline (since it can be run many
> times per packet) So they made the deliberate choice of separating it
> into two functions - one inline, one non-inline.
>
> So my personal feeling is that the code is probably fine as it stands today.
It probably is. Doesn't really matter I guess.
> > Maybe "static" should be removed, too? Oh well.
>
> Uh, why? It's clearly a file-local function.
No real reason, just that none of the other functions in filter.c are static.
Thanks
^ permalink raw reply
* Re: [PATCH] Merge __load_pointer() and load_pointer() in net/core/filter.c; kernel 2.6.14
From: Kris Katterjohn @ 2005-11-03 6:19 UTC (permalink / raw)
To: Mitchell Blank Jr; +Cc: Herbert Xu, jschlst, davem, acme, netdev, linux-kernel
> From: Mitchell Blank Jr
> Cc:
> herbert@gondor.apana.org.au;jschlst@samba.org;davem@davemloft.net;
> acme@ghostprotocols.net;netdev@vger.kernel.org
>
> > (I trimmed the cc: list a bit; no need for this to be on LKML in my opinion)
> >
> > Kris Katterjohn wrote:
> > > Forgive me because this is one of my first attempts at anything related to the
> > > kernel, but...
> > >
> > > 1) How would I go about benchmarking this?
> >
> > The first thing to do is run "size net/core/filter.o" before and after and
> > see if your change makes the "text" section larger.
> >
> > The risk of putting more stuff into the inlined function is just that
> > the rarely-executed path will end up duplicated in a bunch of places, bloating
> > the generated code. It's hard to directly benchmark the effects of this but
> > it will generally make the fast-path code less compact in the L1 I-cache
> > which (if you do it enough) slows everything down. Thats one reason why
> > kernel developers try to keep a close eye on bloat.
> >
> > -Mitch
>
> Before patch:
>
> text data bss dec hex filename
> 2399 0 0 2399 95f x/net/core/filter.o
>
> After patch:
>
> text data bss dec hex filename
> 2589 0 0 2589 a1d y/net/core/filter.o
>
> After patch without inlining the function:
>
> text data bss dec hex filename
> 2127 0 0 2127 84f y/net/core/filter.o
>
>
> So I guess use my patch and take "inline" off? What do you think?
>
>
> Thanks
>
Here's the new patch. It's the same as the first one, just with "inline" removed.
Maybe "static" should be removed, too? Oh well.
---
--- x/net/core/filter.c 2005-10-27 19:02:08.000000000 -0500
+++ y/net/core/filter.c 2005-11-03 00:05:42.000000000 -0600
@@ -13,6 +13,8 @@
* 2 of the License, or (at your option) any later version.
*
* Andi Kleen - Fix a few bad bugs and races.
+ * Kris Katterjohn - Merged __load_pointer() into load_pointer() and
+ * cleaned it up a little bit - 2005-11-01
*/
#include <linux/module.h>
@@ -35,31 +37,27 @@
#include <asm/uaccess.h>
#include <linux/filter.h>
-/* No hurry in this branch */
-static void *__load_pointer(struct sk_buff *skb, int k)
+static void *load_pointer(struct sk_buff *skb, int offset,
+ unsigned int size, void *buffer)
{
u8 *ptr = NULL;
- if (k >= SKF_NET_OFF)
- ptr = skb->nh.raw + k - SKF_NET_OFF;
- else if (k >= SKF_LL_OFF)
- ptr = skb->mac.raw + k - SKF_LL_OFF;
+ if (offset >= 0)
+ return skb_header_pointer(skb, offset, size, buffer);
+
+ if (offset >= SKF_AD_OFF)
+ return NULL;
+
+ if (offset >= SKF_NET_OFF)
+ ptr = skb->nh.raw + offset - SKF_NET_OFF;
+
+ else if (offset >= SKF_LL_OFF)
+ ptr = skb->mac.raw + offset - SKF_LL_OFF;
if (ptr >= skb->head && ptr < skb->tail)
return ptr;
- return NULL;
-}
-static inline void *load_pointer(struct sk_buff *skb, int k,
- unsigned int size, void *buffer)
-{
- if (k >= 0)
- return skb_header_pointer(skb, k, size, buffer);
- else {
- if (k >= SKF_AD_OFF)
- return NULL;
- return __load_pointer(skb, k);
- }
+ return NULL;
}
/**
^ permalink raw reply
* Re: [PATCH]dgrs - Fixes Warnings when CONFIG_ISA and CONFIG_PCI are not enabled
From: Ashutosh Naik @ 2005-11-03 5:39 UTC (permalink / raw)
To: Richard Knutsson
Cc: netdev, davej, acme, linux-net, akpm, linux-kernel, stable
In-Reply-To: <436927CA.3090105@student.ltu.se>
On 11/3/05, Richard Knutsson <ricknu-0@student.ltu.se> wrote:
> >
> >
> >>>This patch fixes compiler warnings when CONFIG_ISA and CONFIG_PCI are
> >>>not enabled in the dgrc network driver.
> >>>
> >>>Signed-off-by: Ashutosh Naik <ashutosh.naik@gmail.com>
> >>>
> >>>--
> >>>diff -Naurp linux-2.6.14/drivers/net/dgrs.c
> >>>linux-2.6.14-git1/drivers/net/dgrs.c---
> >>>linux-2.6.14/drivers/net/dgrs.c 2005-10-28 05:32:08.000000000
> >>>+0530
> >>>+++ linux-2.6.14-git1/drivers/net/dgrs.c 2005-11-01
> >>>10:30:03.000000000 +0530
> >>>@@ -1549,8 +1549,12 @@ MODULE_PARM_DESC(nicmode, "Digi RightSwi
> >>>static int __init dgrs_init_module (void) {
> >>> int i;
> >>>- int eisacount = 0, pcicount = 0;
> >>>-
> >>>+#ifdef CONFIG_EISA
> >>>+ int eisacount = 0;
> >>>+#endif
> >>>+#ifdef CONFIG_PCI
> >>>+ int pcicount = 0;
> >>>+#endif
> >>> /*
> >>> * Command line variable overrides
> >>> * debug=NNN
> >>>-
> >>>
> >>>
>
> >>Signed-off-by: Richard Knutsson <ricknu-0@student.ltu.se>
> >>
> >>---
> >>
> >>diff -uNr a/drivers/net/dgrs.c b/drivers/net/dgrs.c
> >>--- a/drivers/net/dgrs.c 2005-08-29 01:41:01.000000000 +0200
> >>+++ b/drivers/net/dgrs.c 2005-10-26 15:53:43.000000000 +0200
> >>@@ -1549,7 +1549,7 @@
> >> static int __init dgrs_init_module (void)
> >> {
> >> int i;
> >>- int eisacount = 0, pcicount = 0;
> >>+ int count;
> >>
> >> /*
> >> * Command line variable overrides
> >>@@ -1591,14 +1591,14 @@
> >> * Find and configure all the cards
> >> */
> >> #ifdef CONFIG_EISA
> >>- eisacount = eisa_driver_register(&dgrs_eisa_driver);
> >>- if (eisacount < 0)
> >>- return eisacount;
> >>+ count = eisa_driver_register(&dgrs_eisa_driver);
> >>+ if (count < 0)
> >>+ return count;
> >> #endif
> >> #ifdef CONFIG_PCI
> >>- pcicount = pci_register_driver(&dgrs_pci_driver);
> >>- if (pcicount)
> >>- return pcicount;
> >>+ count = pci_register_driver(&dgrs_pci_driver);
> >>+ if (count)
> >>+ return count;
> >> #endif
> >> return 0;
> >> }
> >>
> >>
> >
> >Well, both of them do the same stuff, but one of these patches needs
> >to be committed.
> >
> >Cheers
> >Ashutosh
> >
> >
> Can both CONFIG_PCI and CONFIG_EISA be undefined at the same time? If
> so, I think you patch is better.
> But on line 1015 in the dgrs.c-file (function dgrs_download()) there is
> an if-statement:
> if (priv0->plxreg)
> { /* PCI bus */
> ...
> }
> else
> { /* EISA bus */
> ...
> from where I got the idea it needs either pci or eisa (or both). If this
> is true, I vote for my patch.
Both CONFIG_PCI and CONFIG_EISA cant be undefined at the same time,
because the device has to be on either of the 2 busses. I think your
patch is better in that case.
Cheers
Ashutosh
^ permalink raw reply
* Re: [PATCH][MCAST]IPv6: small fix for ip6_mc_msfilter(...)
From: Willy Tarreau @ 2005-11-03 5:02 UTC (permalink / raw)
To: Marcelo Tosatti
Cc: David Stevens, David S. Miller, linux-kernel, netdev, Yan Zheng
In-Reply-To: <20051102205926.GB5078@logos.cnet>
On Wed, Nov 02, 2005 at 06:59:26PM -0200, Marcelo Tosatti wrote:
> On Wed, Nov 02, 2005 at 09:42:37AM -0800, David Stevens wrote:
> > Willy Tarreau <willy@w.ods.org> wrote on 11/02/2005 01:29:59 AM:
> >
> > > Marcelo, David, does this backport seem appropriate for 2.4.32 ? I
> > verified
> > > that it compiles, nothing more.
> >
> > Yes.
> >
> > > If it's OK, I've noticed another patch that
> > > Yan posted today and which might be of interest before a very solid
> > release.
> >
> > I think they should be reviewed first. :-)
>
> Alright, then let it be tested in v2.6 first. Willy, can you queue for
> 2.4.33-pre ?
OK.
Willy
^ permalink raw reply
* Re: [PKT_SCHED]: Rework QoS and/or fair queueing configuration
From: Arnaldo Carvalho de Melo @ 2005-11-03 4:25 UTC (permalink / raw)
To: Thomas Graf; +Cc: bunk, jengelh, linux-kernel, netdev
In-Reply-To: <20051101141302.GM23537@postel.suug.ch>
On 11/1/05, Thomas Graf <tgraf@suug.ch> wrote:
>
> Make "QoS and/or fair queueing" have its own menu, it's too big to be
> inlined into "Network options". Remove the obsolete NET_QOS option.
> Automatically select NET_CLS if needed. Do the same for NET_ESTIMATOR
> but allow it to be selected manually for statistical purposes. Add
> comments to separate queueing from classification. Fix dependencies
> and ordering of classifiers. Improve descriptions/help texts and
> remove outdated pieces.
>
> Signed-off-by: Thomas Graf <tgraf@suug.ch>
Thanks, applied.
^ permalink raw reply
* Re: [PATCH] Merge __load_pointer() and load_pointer() in net/core/filter.c; kernel 2.6.14
From: Kris Katterjohn @ 2005-11-03 2:48 UTC (permalink / raw)
To: Herbert Xu; +Cc: jschlst, torvalds, linux-kernel, davem, acme, netdev
Forgive me because this is one of my first attempts at anything related to the
kernel, but...
1) How would I go about benchmarking this?
2) If the out-of-line code is executed anyway (since load_pointer() calls __load_pointer()),
would it really effect it very much?
I've been programming as a hobby for a few years, but I never really worried
about inlining my code. Most likely because when I was learning C, I was using
books etc. that were written before C99 was out.
Thanks
----- Original Message -----
From: Herbert Xu [mailto:herbert@gondor.apana.org.au]
Subject: Re: [PATCH] Merge __load_pointer() and load_pointer() in net/core/filter.c; kernel 2.6.14
> Kris Katterjohn <kjak@ispwest.com> wrote:
> > I wasn't actually changing it to add performance, but to make the code look
> > cleaner. The new load_pointer() is virtually the same as having the seperate
> > functions that are currently there, but the code, I think, is "better looking".
> > If you look at the current net/core/filter.c and then my patched version, the
> > steps are done in the exact same order and same way, but all in that one
> > function.
>
> You've just changed an out-of-line function (__load_pointer) into an
> inlined function. There may be a cost to that.
> --
> Visit Openswan at http://www.openswan.org/
> Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
> Home Page: http://gondor.apana.org.au/~herbert/
> PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply
* Re: [PATCH] Merge __load_pointer() and load_pointer() in net/core/filter.c; kernel 2.6.14
From: Herbert Xu @ 2005-11-03 2:30 UTC (permalink / raw)
To: Kris Katterjohn
Cc: herbert, jschlst, torvalds, linux-kernel, davem, acme, netdev
In-Reply-To: <310a17ad00f84c389e64ae26656ce1a1.kjak@ispwest.com>
Kris Katterjohn <kjak@ispwest.com> wrote:
> I wasn't actually changing it to add performance, but to make the code look
> cleaner. The new load_pointer() is virtually the same as having the seperate
> functions that are currently there, but the code, I think, is "better looking".
> If you look at the current net/core/filter.c and then my patched version, the
> steps are done in the exact same order and same way, but all in that one
> function.
You've just changed an out-of-line function (__load_pointer) into an
inlined function. There may be a cost to that.
--
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply
* Re: [PATCH] Merge __load_pointer() and load_pointer() in net/core/filter.c; kernel 2.6.14
From: Kris Katterjohn @ 2005-11-03 2:13 UTC (permalink / raw)
To: Herbert Xu; +Cc: jschlst, torvalds, linux-kernel, davem, acme, netdev
I wasn't actually changing it to add performance, but to make the code look
cleaner. The new load_pointer() is virtually the same as having the seperate
functions that are currently there, but the code, I think, is "better looking".
If you look at the current net/core/filter.c and then my patched version, the
steps are done in the exact same order and same way, but all in that one
function.
That may sound silly, but I just kept looking at it and asking myself, "Why?"
Of course, one way may still out-perform the other.
Thanks
----- Original Message -----
From: Herbert Xu [mailto:herbert@gondor.apana.org.au]
Sent: 11/2/2005 5:50:29 PM
To: kjak@users.sourceforge.net
Cc: jschlst@samba.org; torvalds@osdl.org; linux-kernel@vger.kernel.org; davem@davemloft.net; acme@ghostprotocols.net; netdev@vger.kernel.org
Subject: Re: [PATCH] Merge __load_pointer() and load_pointer() in net/core/filter.c; kernel 2.6.14
> Kris Katterjohn <kjak@ispwest.com> wrote:
> > Another patch for net/core/filter.c by me. I merged __load_pointer() into
> > load_pointer(). I don't see a point in having two seperate functions when
> > both functions are really small and load_pointer() calls __load_pointer().
> > Renamed the variable "k" to "offset" since that's what it is, and in
> > skb_header_pointer() it's "offset".
> >
> > This patch is a diff from kernel 2.6.14
>
> This code path is performance-critical so you should benchmark your
> changes.
>
> In this particular case, __load_pointer is the unlikely case and
> therefore it wasn't inlined.
>
> Cheers,
> --
> Visit Openswan at http://www.openswan.org/
> Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
> Home Page: http://gondor.apana.org.au/~herbert/
> PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply
* Re: [PATCH] Merge __load_pointer() and load_pointer() in net/core/filter.c; kernel 2.6.14
From: Herbert Xu @ 2005-11-03 1:50 UTC (permalink / raw)
To: Kris Katterjohn; +Cc: jschlst, torvalds, linux-kernel, davem, acme, netdev
In-Reply-To: <e626300280b94e0abc26defcc6043b91.kjak@ispwest.com>
Kris Katterjohn <kjak@ispwest.com> wrote:
> Another patch for net/core/filter.c by me. I merged __load_pointer() into
> load_pointer(). I don't see a point in having two seperate functions when
> both functions are really small and load_pointer() calls __load_pointer().
> Renamed the variable "k" to "offset" since that's what it is, and in
> skb_header_pointer() it's "offset".
>
> This patch is a diff from kernel 2.6.14
This code path is performance-critical so you should benchmark your
changes.
In this particular case, __load_pointer is the unlikely case and
therefore it wasn't inlined.
Cheers,
--
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply
* Re: [PATCH][MCAST]Two fix for implementation of MLDv2 .
From: Yan Zheng @ 2005-11-03 1:22 UTC (permalink / raw)
To: Yan Zheng; +Cc: netdev, linux-kernel, David Stevens
In-Reply-To: <436878E7.3030303@21cn.com>
I find something interesting . Maybe more changes for is_in(...) are needed.
static int is_in(struct ifmcaddr6 *pmc, struct ip6_sf_list *psf, int type,
int gdeleted, int sdeleted)
{
switch (type) {
case MLD2_MODE_IS_INCLUDE:
case MLD2_CHANGE_TO_INCLUDE:
if (gdeleted || sdeleted)
return 0;
if (psf->sf_count[MCAST_INCLUDE] == 0)
return 0; // maybe never happen
if (type == MLD2_CHANGE_TO_INCLUDE)
return 1;
return !((pmc->mca_flags & MAF_GSQUERY) && !psf->sf_gsresp);
case MLD2_MODE_IS_EXCLUDE:
case MLD2_CHANGE_TO_EXCLUDE:
if (gdeleted || sdeleted)
return 0;
if (pmc->mca_sfcount[MCAST_EXCLUDE] == 0 ||
psf->sf_count[MCAST_INCLUDE])
return 0;
if (type == MLD2_MODE_IS_EXCLUDE)
return 1;
return pmc->mca_sfcount[MCAST_EXCLUDE] ==
psf->sf_count[MCAST_EXCLUDE];
case MLD2_ALLOW_NEW_SOURCES:
if (gdeleted || !psf->sf_crcount)
return 0;
return (pmc->mca_sfmode == MCAST_INCLUDE) ^ sdeleted;
case MLD2_BLOCK_OLD_SOURCES:
if (pmc->mca_sfmode == MCAST_INCLUDE)
return gdeleted || (psf->sf_crcount && sdeleted);
return psf->sf_crcount && !gdeleted && !sdeleted;
}
return 0;
}
^ permalink raw reply
* Re: [PATCH]A old patch for addrconf_ifdown(...).
From: Arnaldo Carvalho de Melo @ 2005-11-03 1:14 UTC (permalink / raw)
To: Yan Zheng; +Cc: netdev, linux-kernel
In-Reply-To: <43695E77.3080707@21cn.com>
On 11/2/05, Yan Zheng <yanzheng@21cn.com> wrote:
> Hi.
>
> The patch may be got lost. It's already acked by YOSHIFUJI.
>
>
> Signed-off-by: Yan Zheng<yanzheng@21cn.com>
I tought it had already been applied by Dave:
http://www.uwsg.iu.edu/hypermail/linux/kernel/0510.1/1290.html
Anyway, applying it now, thanks.
- Arnaldo
^ permalink raw reply
* [PATCH]A old patch for addrconf_ifdown(...).
From: Yan Zheng @ 2005-11-03 0:48 UTC (permalink / raw)
To: netdev; +Cc: linux-kernel, Arnaldo Carvalho de Melo
Hi.
The patch may be got lost. It's already acked by YOSHIFUJI.
Signed-off-by: Yan Zheng<yanzheng@21cn.com>
Index: net/ipv6/addrconf.c
------------------------------------------------------------------------
--- linux-2.6.14/net/ipv6/addrconf.c 2005-10-28 08:02:08.000000000 +0800
+++ linux/net/ipv6/addrconf.c 2005-11-03 08:35:11.000000000 +0800
@@ -2167,7 +2167,7 @@ static int addrconf_ifdown(struct net_de
/* Step 5: netlink notification of this interface */
idev->tstamp = jiffies;
- inet6_ifinfo_notify(RTM_NEWLINK, idev);
+ inet6_ifinfo_notify(RTM_DELLINK, idev);
/* Shot the device (if unregistered) */
^ permalink raw reply
* Re: [PATCH][MCAST]IPv6: small fix for ip6_mc_msfilter(...)
From: Arnaldo Carvalho de Melo @ 2005-11-02 23:28 UTC (permalink / raw)
To: Yan Zheng; +Cc: netdev, linux-kernel, David Stevens
In-Reply-To: <43694B94.7010605@21cn.com>
On 11/2/05, Yan Zheng <yanzheng@21cn.com> wrote:
> >
> > Could you please compile test it next time :-) hint, missing ';'.
> > Anyway, fixed up by hand.
> >
> > - Arnaldo
> >
> >
>
> I'm so sorry.
Don't worry, I already fixed it and published in my net-2.6.git tree at
www.kernel.org, its just that the same confidence that makes one
commit something simple as in your case could introduce something
nasty :-)
- Arnaldo
^ permalink raw reply
* Re: [PATCH][MCAST]IPv6: small fix for ip6_mc_msfilter(...)
From: Yan Zheng @ 2005-11-02 23:28 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo; +Cc: netdev, linux-kernel, David Stevens
In-Reply-To: <39e6f6c70511021355i52aff7e4n19ca4c1e24b21bb7@mail.gmail.com>
>
> Could you please compile test it next time :-) hint, missing ';'.
> Anyway, fixed up by hand.
>
> - Arnaldo
>
>
I'm so sorry.
============================================================
--- linux-2.6.14/net/ipv4/igmp.c 2005-10-28 08:02:08.000000000 +0800
+++ linux/net/ipv4/igmp.c 2005-11-02 07:31:01.000000000 +0800
@@ -1908,8 +1908,11 @@ int ip_mc_msfilter(struct sock *sk, stru
sock_kfree_s(sk, newpsl, IP_SFLSIZE(newpsl->sl_max));
goto done;
}
- } else
+ } else {
newpsl = NULL;
+ (void) ip_mc_add_src(in_dev, &msf->imsf_multiaddr,
+ msf->imsf_fmode, 0, NULL, 0);
+ }
psl = pmc->sflist;
if (psl) {
(void) ip_mc_del_src(in_dev, &msf->imsf_multiaddr, pmc->sfmode,
^ permalink raw reply
* Re: [PATCH][MCAST]IPv6: small fix for ip6_mc_msfilter(...)
From: Arnaldo Carvalho de Melo @ 2005-11-02 21:55 UTC (permalink / raw)
To: Yan Zheng; +Cc: netdev, linux-kernel, David Stevens
In-Reply-To: <4367FF22.3030601@21cn.com>
On 11/1/05, Yan Zheng <yanzheng@21cn.com> wrote:
> David Stevens wrote:
> > Yan,
> > Please also make this equivalent change in IPv4 with
> > ip_mc_msfilter() and ip_mc_add_src().
> >
> > +-DLS
> >
> > Acked-by: David L Stevens <dlstevens@us.ibm.com>
>
> To keep code style, I also create a new patch for IPv6. :-)
>
> Signed-off-by: Yan Zheng <yanzheng@21cn.com>
>
> Patch for IPv4
> Index:net/ipv4/igmp.c
> ============================================================
> --- linux-2.6.14/net/ipv4/igmp.c 2005-10-28 08:02:08.000000000 +0800
> +++ linux/net/ipv4/igmp.c 2005-11-02 07:31:01.000000000 +0800
> @@ -1908,8 +1908,11 @@ int ip_mc_msfilter(struct sock *sk, stru
> sock_kfree_s(sk, newpsl, IP_SFLSIZE(newpsl->sl_max));
> goto done;
> }
> - } else
> + } else {
> newpsl = NULL;
> + (void) ip_mc_add_src(in_dev, &msf->imsf_multiaddr,
> + msf->imsf_fmode, 0, NULL, 0)
Could you please compile test it next time :-) hint, missing ';'.
Anyway, fixed up by hand.
- Arnaldo
^ permalink raw reply
* Re: [PATCH][MCAST]IPv6: small fix for ip6_mc_msfilter(...)
From: Arnaldo Carvalho de Melo @ 2005-11-02 21:46 UTC (permalink / raw)
To: David Stevens
Cc: Willy Tarreau, David S. Miller, linux-kernel, Marcelo Tosatti,
netdev, Yan Zheng
In-Reply-To: <OF9D4BE592.A4BBC034-ON882570AD.00608386-882570AD.006143BA@us.ibm.com>
On 11/2/05, David Stevens <dlstevens@us.ibm.com> wrote:
> Willy Tarreau <willy@w.ods.org> wrote on 11/02/2005 01:29:59 AM:
>
> > Marcelo, David, does this backport seem appropriate for 2.4.32 ? I
> verified
> > that it compiles, nothing more.
>
> Yes.
>
> > If it's OK, I've noticed another patch that
> > Yan posted today and which might be of interest before a very solid
> release.
>
> I think they should be reviewed first. :-)
Sure thing David, and thanks for your much appreciated review of patches for
the area.
- Arnaldo
^ permalink raw reply
* Re: [PATCH][MCAST]IPv6: small fix for ip6_mc_msfilter(...)
From: Arnaldo Carvalho de Melo @ 2005-11-02 21:45 UTC (permalink / raw)
To: David Stevens; +Cc: Yan Zheng, linux-kernel, netdev
In-Reply-To: <OF19AE07D9.BF2B28D4-ON882570AC.0083498D-882570AC.00835D3A@us.ibm.com>
On 11/1/05, David Stevens <dlstevens@us.ibm.com> wrote:
> Looks good. Thanks, Yan.
>
> +-DLS
Thanks everybody, Applied.
- Arnaldo
^ permalink raw reply
* [PATCH-2.4][MCAST]IPv6: small fix for ip6_mc_msfilter(...)
From: Willy Tarreau @ 2005-11-02 21:31 UTC (permalink / raw)
To: Marcelo Tosatti
Cc: David Stevens, linux-kernel, netdev, Yan Zheng, David S. Miller
In-Reply-To: <20051102134112.GB2609@logos.cnet>
Hi Marcelo,
On Wed, Nov 02, 2005 at 11:41:12AM -0200, Marcelo Tosatti wrote:
> Given the fact that it is a bug correction, sure.
> Could you please prepare a nice e-mail with the full description?
Yes, I see... email was not nice enough to the scripts :-)
Here it is complete and clean below.
Regards,
Willy
[PATCH-2.4][MCAST]IPv6: small fix for ip6_mc_msfilter(...)
Multicast source filters aren't widely used yet, and that's really
the only feature that's affected if an application actually exercises
this bug, as far as I can tell. An ordinary filter-less multicast join
should still work, and only forwarded multicast traffic making use of
filters and doing empty-source filters with the MSFILTER ioctl would
be at risk of not getting multicast traffic forwarded to them because
the reports generated would not be based on the correct counts.
Initial 2.6 patch by Yan Zheng, bug explanation by David Stevens,
patch ACKed by David.
Signed-off-by: Willy Tarreau <willy@w.ods.org>
diff -urN linux-2.4.32-rc2/net/ipv4/igmp.c linux-2.4.32-rc2-mcast/net/ipv4/igmp.c
--- linux-2.4.32-rc2/net/ipv4/igmp.c 2005-11-02 10:16:03.000000000 +0100
+++ linux-2.4.32-rc2-mcast/net/ipv4/igmp.c 2005-11-02 10:20:33.000000000 +0100
@@ -1876,8 +1876,11 @@
sock_kfree_s(sk, newpsl, IP_SFLSIZE(newpsl->sl_max));
goto done;
}
- } else
- newpsl = 0;
+ } else {
+ newpsl = NULL;
+ (void) ip_mc_add_src(in_dev, &msf->imsf_multiaddr,
+ msf->imsf_fmode, 0, NULL, 0);
+ }
psl = pmc->sflist;
if (psl) {
(void) ip_mc_del_src(in_dev, &msf->imsf_multiaddr, pmc->sfmode,
diff -urN linux-2.4.32-rc2/net/ipv6/mcast.c linux-2.4.32-rc2-mcast/net/ipv6/mcast.c
--- linux-2.4.32-rc2/net/ipv6/mcast.c 2005-11-02 10:16:03.000000000 +0100
+++ linux-2.4.32-rc2-mcast/net/ipv6/mcast.c 2005-11-02 10:21:49.000000000 +0100
@@ -505,8 +505,11 @@
sock_kfree_s(sk, newpsl, IP6_SFLSIZE(newpsl->sl_max));
goto done;
}
- } else
- newpsl = 0;
+ } else {
+ newpsl = NULL;
+ (void) ip6_mc_add_src(idev, group, gsf->gf_fmode, 0, NULL, 0);
+ }
+
psl = pmc->sflist;
if (psl) {
(void) ip6_mc_del_src(idev, group, pmc->sfmode,
^ permalink raw reply
* Re: [PATCH 2.6.13] airo.c: remove delay in airo_get_scan
From: Michal Schmidt @ 2005-11-02 21:29 UTC (permalink / raw)
To: Bill Moss; +Cc: linville, netdev, linux-kernel
In-Reply-To: <200511020347.jA23lvi9028538@localhost.localdomain>
Bill Moss wrote:
> Remove 3 second delay in airo_get_scan. Testing shows this delay is unnecessary. Users of NetworkManager
> find this delay make NetworkManager slow to connect.
>
> --- a/drivers/net/wireless/airo.c 2005-11-01 21:21:04.000000000 -0500
> +++ b/drivers/net/wireless/airo.c 2005-11-01 21:22:41.000000000 -0500
> @@ -6918,7 +6918,7 @@
> /* When we are associated again, the scan has surely finished.
> * Just in case, let's make sure enough time has elapsed since
> * we started the scan. - Javier */
> - if(ai->scan_timestamp && time_before(jiffies,ai->scan_timestamp+3*HZ)) {
> + if(ai->scan_timestamp && time_before(jiffies,ai->scan_timestamp)) {
> /* Important note : we don't want to block the caller
> * until results are ready for various reasons.
> * First, managing wait queues is complex and racy
>
> Signed-off-by: Bill Moss <bmoss@clemson.edu>
With this change, the time_before() test can't possibly succeed, can it?
In that case, why not remove the if and its body completely?
Michal
^ 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