* Re: [PATCH] ibmveth fix panic in initial replenish cycle
From: John W. Linville @ 2005-11-04 22:23 UTC (permalink / raw)
To: Santiago Leon; +Cc: Linus Torvalds, netdev, lkml, Jeff Garzik
In-Reply-To: <20051101175617.25145.73324.sendpatchset@ltcml8p7.rchland.ibm.com>
On Tue, Nov 01, 2005 at 02:15:09PM -0500, Santiago Leon wrote:
> This patch fixes a panic in the current tree caused by a race condition between the initial replenish cycle and the rx processing of the first packets trying to replenish the buffers.
Please restrain your line widths to less than 80 characters. 72 is
a nice number IMHO.
Thanks,
John
--
John W. Linville
linville@tuxdriver.com
^ permalink raw reply
* Re: ancient ieee80211/ipw2200 drivers in recent kernel (2.6.14)
From: Christoph Hellwig @ 2005-11-05 0:21 UTC (permalink / raw)
To: Mikhail Gusarov, debian-kernel, NetDev
In-Reply-To: <20051104032918.GE4708@verge.net.au>
On Fri, Nov 04, 2005 at 12:29:20PM +0900, Horms wrote:
> do I take that comment to mean that upstream can't update the
> drivers but Debian can? And if so, do you recommend updating
> Debian's kernel packages, or putting the updates elsewhere?
Well, we could upstream, but so far no one is annoyed enough to
overrid the driver maintainer. I'd suggest merging a more recent
driver into the debian kernel package.
^ permalink raw reply
* Re: [PATCH] [IPV4] Fix secondary IP addresses after promotion
From: Patrick McHardy @ 2005-11-05 0:34 UTC (permalink / raw)
To: Brian Pomerantz
Cc: netdev, davem, kuznet, pekkas, jmorris, yoshfuji, kaber,
linux-kernel
In-Reply-To: <20051104184633.GA16256@skull.piratehaven.org>
[-- Attachment #1: Type: text/plain, Size: 1427 bytes --]
Brian Pomerantz wrote:
> When 3 or more IP addresses in the same subnet exist on a device and the
> first one is removed, only the promoted IP address can be reached. Just
> after promotion of the next IP address, this fix spins through any more
> IP addresses on the interface and sends a NETDEV_UP notification for
> that address. This repopulates the FIB with the proper route
> information.
>
> @@ -294,7 +294,13 @@ static void inet_del_ifa(struct in_devic
> /* not sure if we should send a delete notify first? */
> promote->ifa_flags &= ~IFA_F_SECONDARY;
> rtmsg_ifa(RTM_NEWADDR, promote);
> - notifier_call_chain(&inetaddr_chain, NETDEV_UP, promote);
> +
> + /* update fib in the rest of this address list */
> + ifa = promote;
> + while (ifa != NULL) {
> + notifier_call_chain(&inetaddr_chain, NETDEV_UP, ifa);
> + ifa = ifa->ifa_next;
> + }
> }
> }
You assume all addresses following the primary addresses are secondary
addresses of the primary, which is not true with multiple primaries.
This patch (untested) makes sure only to send notification for real
secondaries of the deleted address. It also removes a racy double-
check for IN_DEV_PROMOTE_SECONDARIES - once we've decided to promote
an address checking again opens a window in which address promotion
could be disabled and we end up with only secondaries without a
primary address.
Signed-off-by: Patrick McHardy <kaber@trash.net>
[-- Attachment #2: x --]
[-- Type: text/plain, Size: 1535 bytes --]
diff --git a/net/ipv4/devinet.c b/net/ipv4/devinet.c
index 4ec4b2c..beb02cc 100644
--- a/net/ipv4/devinet.c
+++ b/net/ipv4/devinet.c
@@ -234,7 +234,7 @@ static void inet_del_ifa(struct in_devic
int destroy)
{
struct in_ifaddr *promote = NULL;
- struct in_ifaddr *ifa1 = *ifap;
+ struct in_ifaddr *ifa, *ifa1 = *ifap;
ASSERT_RTNL();
@@ -243,7 +243,6 @@ static void inet_del_ifa(struct in_devic
**/
if (!(ifa1->ifa_flags & IFA_F_SECONDARY)) {
- struct in_ifaddr *ifa;
struct in_ifaddr **ifap1 = &ifa1->ifa_next;
while ((ifa = *ifap1) != NULL) {
@@ -283,19 +282,25 @@ static void inet_del_ifa(struct in_devic
*/
rtmsg_ifa(RTM_DELADDR, ifa1);
notifier_call_chain(&inetaddr_chain, NETDEV_DOWN, ifa1);
+
+ if (promote) {
+ /* not sure if we should send a delete notify first? */
+ promote->ifa_flags &= ~IFA_F_SECONDARY;
+ rtmsg_ifa(RTM_NEWADDR, promote);
+ for (ifa = promote; ifa; ifa = ifa->ifa_next) {
+ if (ifa1->ifa_mask != ifa->ifa_mask ||
+ !inet_ifa_match(ifa1->ifa_address, ifa))
+ continue;
+ notifier_call_chain(&inetaddr_chain, NETDEV_UP, ifa);
+ }
+ }
+
if (destroy) {
inet_free_ifa(ifa1);
if (!in_dev->ifa_list)
inetdev_destroy(in_dev);
}
-
- if (promote && IN_DEV_PROMOTE_SECONDARIES(in_dev)) {
- /* not sure if we should send a delete notify first? */
- promote->ifa_flags &= ~IFA_F_SECONDARY;
- rtmsg_ifa(RTM_NEWADDR, promote);
- notifier_call_chain(&inetaddr_chain, NETDEV_UP, promote);
- }
}
static int inet_insert_ifa(struct in_ifaddr *ifa)
^ permalink raw reply related
* Re: [PATCH] [IPV4] Fix secondary IP addresses after promotion
From: Brian Pomerantz @ 2005-11-05 0:58 UTC (permalink / raw)
To: Patrick McHardy
Cc: netdev, davem, kuznet, pekkas, jmorris, yoshfuji, kaber,
linux-kernel
In-Reply-To: <436BFE08.6030906@trash.net>
On Sat, Nov 05, 2005 at 01:34:16AM +0100, Patrick McHardy wrote:
>
> You assume all addresses following the primary addresses are secondary
> addresses of the primary, which is not true with multiple primaries.
> This patch (untested) makes sure only to send notification for real
> secondaries of the deleted address. It also removes a racy double-
> check for IN_DEV_PROMOTE_SECONDARIES - once we've decided to promote
> an address checking again opens a window in which address promotion
> could be disabled and we end up with only secondaries without a
> primary address.
>
Yeah, I was wondering if there could be primaries after the
secondaries. I'm pretty unfamiliar with this code (first looked at it
last week) and still don't have a handle on how the primaries
interact with the secondaries in the route lookup. Which means it's
not clear to me why this was failing to begin with. :)
Your patch works for all of the cases I've been testing with so it
looks good to go from here.
BAPper
^ permalink raw reply
* Re: [PATCH] [IPV4] Fix secondary IP addresses after promotion
From: Thomas Graf @ 2005-11-05 1:07 UTC (permalink / raw)
To: Patrick McHardy
Cc: Brian Pomerantz, netdev, davem, kuznet, pekkas, jmorris, yoshfuji,
kaber, linux-kernel
In-Reply-To: <436BFE08.6030906@trash.net>
* Patrick McHardy <kaber@trash.net> 2005-11-05 01:34
> Brian Pomerantz wrote:
> >When 3 or more IP addresses in the same subnet exist on a device and the
> >first one is removed, only the promoted IP address can be reached. Just
> >after promotion of the next IP address, this fix spins through any more
> >IP addresses on the interface and sends a NETDEV_UP notification for
> >that address. This repopulates the FIB with the proper route
> >information.
> >
> >@@ -294,7 +294,13 @@ static void inet_del_ifa(struct in_devic
> > /* not sure if we should send a delete notify first? */
> > promote->ifa_flags &= ~IFA_F_SECONDARY;
> > rtmsg_ifa(RTM_NEWADDR, promote);
> >- notifier_call_chain(&inetaddr_chain, NETDEV_UP, promote);
> >+
> >+ /* update fib in the rest of this address list */
> >+ ifa = promote;
> >+ while (ifa != NULL) {
> >+ notifier_call_chain(&inetaddr_chain, NETDEV_UP, ifa);
> >+ ifa = ifa->ifa_next;
> >+ }
> > }
> > }
>
> You assume all addresses following the primary addresses are secondary
> addresses of the primary, which is not true with multiple primaries.
> This patch (untested) makes sure only to send notification for real
> secondaries of the deleted address.
Even this corrected version is only a workaround, the real bug is that
or whatever reason all local routes of seconaries get deleted upon an
address promotion. I started debugging it a bit by looking at the
requests generated by fib_magic() and the resulting notifications, the
local routes just disappear when they shouldn't.
Situation is: 10.0.0.[1-4]/24 on dev0, 10.0.0.1 is the primary address
and gets deleted while address promotion is enabled. The following
happens:
[Format:]
Request generated by fib_magic()
Notification event received
RTM_DELROUTE 10.0.0.0/24 dev eth0 scope link
unicast table main protocol 2 preferred-src 10.0.0.1
RTM_DELROUTE 10.0.0.0/24 dev eth0 scope link
unicast table main protocol 2 preferred-src 10.0.0.1
RTM_DELROUTE 10.0.0.255 dev eth0 scope link
broadcast table local protocol 2 preferred-src 10.0.0.1
RTM_DELROUTE 10.0.0.255 dev eth0 scope link
broadcast table local protocol 2 preferred-src 10.0.0.1
RTM_DELROUTE 10.0.0.0 dev eth0 scope link
broadcast table local protocol 2 preferred-src 10.0.0.1
RTM_DELROUTE 10.0.0.0 dev eth0 scope link
broadcast table local protocol 2 preferred-src 10.0.0.1
RTM_DELROUTE 10.0.0.1 dev eth0 scope host
local table local protocol 2 preferred-src 10.0.0.1
RTM_DELROUTE 10.0.0.1 dev eth0 scope host
local table local protocol 2 preferred-src 10.0.0.1
RTM_NEWROUTE 10.0.0.2 dev eth0 scope host
local table local protocol 2 preferred-src 10.0.0.2
RTM_NEWROUTE 10.0.0.2 dev eth0 scope host
local table local protocol 2 preferred-src 10.0.0.2
RTM_NEWROUTE 10.0.0.0/24 dev eth0 scope link
unicast table main protocol 2 preferred-src 10.0.0.2
RTM_NEWROUTE 10.0.0.0/24 dev eth0 scope link
unicast table main protocol 2 preferred-src 10.0.0.2
RTM_NEWROUTE 10.0.0.0 dev eth0 scope link
broadcast table local protocol 2 preferred-src 10.0.0.2
RTM_NEWROUTE 10.0.0.0 dev eth0 scope link
broadcast table local protocol 2 preferred-src 10.0.0.2
RTM_NEWROUTE 10.0.0.255 dev eth0 scope link
broadcast table local protocol 2 preferred-src 10.0.0.2
RTM_NEWROUTE 10.0.0.255 dev eth0 scope link
broadcast table local protocol 2 preferred-src 10.0.0.2
State afterwards:
4: eth0: <BROADCAST,MULTICAST,UP> mtu 1500 qdisc pfifo_fast qlen 1000
inet 10.0.0.2/24 scope global eth0
inet 10.0.0.3/24 scope global secondary eth0
inet 10.0.0.4/24 scope global secondary eth0
broadcast 10.0.0.0 proto kernel scope link src 10.0.0.2
local 10.0.0.2 proto kernel scope host src 10.0.0.2
broadcast 10.0.0.255 proto kernel scope link src 10.0.0.2
Local routes for 10.0.0.3 and 10.0.0.4 have disappeared _without_
any notification.
I think the correct way to fix this is to prevent the deletion of
the local routes, not just readding them. _If_ the deletion of them
is intended, which I doubt, then at least notifications must be
sent out.
Code to get fib_magic() requests to userspace:
Index: linux-2.6/include/linux/rtnetlink.h
===================================================================
--- linux-2.6.orig/include/linux/rtnetlink.h
+++ linux-2.6/include/linux/rtnetlink.h
@@ -880,6 +880,8 @@ enum rtnetlink_groups {
#define RTNLGRP_DECnet_ROUTE RTNLGRP_DECnet_ROUTE
RTNLGRP_IPV6_PREFIX,
#define RTNLGRP_IPV6_PREFIX RTNLGRP_IPV6_PREFIX
+ RTNLGRP_FIB_MAGIC,
+#define RTNLGRP_FIB_MAGIC RTNLGRP_FIB_MAGIC
__RTNLGRP_MAX
};
#define RTNLGRP_MAX (__RTNLGRP_MAX - 1)
Index: linux-2.6/net/ipv4/fib_frontend.c
===================================================================
--- linux-2.6.orig/net/ipv4/fib_frontend.c
+++ linux-2.6/net/ipv4/fib_frontend.c
@@ -359,6 +359,48 @@ int inet_dump_fib(struct sk_buff *skb, s
return skb->len;
}
+static int fib_magic_build(struct sk_buff *skb, int type, struct nlmsghdr *nlh,
+ struct rtmsg *rtm, struct kern_rta *rta)
+{
+ struct nlmsghdr *dst = NULL;
+ struct rtmsg *rtm_dst;
+
+ dst = NLMSG_NEW(skb, current->pid, 0, type, sizeof(*rtm), 0);
+ memcpy(dst, nlh, sizeof(*nlh));
+
+ rtm_dst = NLMSG_DATA(dst);
+ memcpy(rtm_dst, rtm, sizeof(*rtm));
+ rtm_dst->rtm_family = AF_INET;
+
+ RTA_PUT(skb, RTA_DST, 4, rta->rta_dst);
+ RTA_PUT(skb, RTA_PREFSRC, 4, rta->rta_prefsrc);
+ RTA_PUT(skb, RTA_OIF, 4, rta->rta_oif);
+
+ return NLMSG_END(skb, dst);
+rtattr_failure:
+nlmsg_failure:
+ return NLMSG_CANCEL(skb, dst);
+}
+
+static void fib_magic_event(int type, struct nlmsghdr *nlh, struct rtmsg *rtm,
+ struct kern_rta *rta)
+{
+ struct sk_buff *skb;
+
+ skb = alloc_skb(NLMSG_SPACE(sizeof(struct rtmsg) + 256), GFP_KERNEL);
+ if (!skb)
+ return;
+
+ if (fib_magic_build(skb, type, nlh, rtm, rta) < 0) {
+ kfree_skb(skb);
+ return;
+ }
+
+ NETLINK_CB(skb).dst_group = RTNLGRP_FIB_MAGIC;
+ netlink_broadcast(rtnl, skb, 0, RTNLGRP_FIB_MAGIC, GFP_KERNEL);
+}
+
+
/* Prepare and feed intra-kernel routing request.
Really, it should be netlink message, but :-( netlink
can be not configured, so that we feed it directly
@@ -402,6 +444,8 @@ static void fib_magic(int cmd, int type,
rta.rta_prefsrc = &ifa->ifa_local;
rta.rta_oif = &ifa->ifa_dev->dev->ifindex;
+ fib_magic_event(cmd, &req.nlh, &req.rtm, &rta);
+
if (cmd == RTM_NEWROUTE)
tb->tb_insert(tb, &req.rtm, &rta, &req.nlh, NULL);
else
^ permalink raw reply
* Re: [PATCH] [IPV4] Fix secondary IP addresses after promotion
From: Patrick McHardy @ 2005-11-05 1:21 UTC (permalink / raw)
To: Thomas Graf
Cc: Brian Pomerantz, netdev, davem, kuznet, pekkas, jmorris, yoshfuji,
kaber, linux-kernel
In-Reply-To: <20051105010740.GR23537@postel.suug.ch>
Thomas Graf wrote:
> * Patrick McHardy <kaber@trash.net> 2005-11-05 01:34
>
>>You assume all addresses following the primary addresses are secondary
>>addresses of the primary, which is not true with multiple primaries.
>>This patch (untested) makes sure only to send notification for real
>>secondaries of the deleted address.
>
>
> Even this corrected version is only a workaround, the real bug is that
> or whatever reason all local routes of seconaries get deleted upon an
> address promotion. I started debugging it a bit by looking at the
> requests generated by fib_magic() and the resulting notifications, the
> local routes just disappear when they shouldn't.
>
> Situation is: 10.0.0.[1-4]/24 on dev0, 10.0.0.1 is the primary address
> and gets deleted while address promotion is enabled. The following
> happens:
>
> [Format:]
> Request generated by fib_magic()
> Notification event received
>
> RTM_DELROUTE 10.0.0.0/24 dev eth0 scope link
> unicast table main protocol 2 preferred-src 10.0.0.1
> RTM_DELROUTE 10.0.0.0/24 dev eth0 scope link
> unicast table main protocol 2 preferred-src 10.0.0.1
>
> RTM_DELROUTE 10.0.0.255 dev eth0 scope link
> broadcast table local protocol 2 preferred-src 10.0.0.1
> RTM_DELROUTE 10.0.0.255 dev eth0 scope link
> broadcast table local protocol 2 preferred-src 10.0.0.1
>
> RTM_DELROUTE 10.0.0.0 dev eth0 scope link
> broadcast table local protocol 2 preferred-src 10.0.0.1
> RTM_DELROUTE 10.0.0.0 dev eth0 scope link
> broadcast table local protocol 2 preferred-src 10.0.0.1
>
> RTM_DELROUTE 10.0.0.1 dev eth0 scope host
> local table local protocol 2 preferred-src 10.0.0.1
> RTM_DELROUTE 10.0.0.1 dev eth0 scope host
> local table local protocol 2 preferred-src 10.0.0.1
>
> RTM_NEWROUTE 10.0.0.2 dev eth0 scope host
> local table local protocol 2 preferred-src 10.0.0.2
> RTM_NEWROUTE 10.0.0.2 dev eth0 scope host
> local table local protocol 2 preferred-src 10.0.0.2
>
> RTM_NEWROUTE 10.0.0.0/24 dev eth0 scope link
> unicast table main protocol 2 preferred-src 10.0.0.2
> RTM_NEWROUTE 10.0.0.0/24 dev eth0 scope link
> unicast table main protocol 2 preferred-src 10.0.0.2
>
> RTM_NEWROUTE 10.0.0.0 dev eth0 scope link
> broadcast table local protocol 2 preferred-src 10.0.0.2
> RTM_NEWROUTE 10.0.0.0 dev eth0 scope link
> broadcast table local protocol 2 preferred-src 10.0.0.2
>
> RTM_NEWROUTE 10.0.0.255 dev eth0 scope link
> broadcast table local protocol 2 preferred-src 10.0.0.2
> RTM_NEWROUTE 10.0.0.255 dev eth0 scope link
> broadcast table local protocol 2 preferred-src 10.0.0.2
>
> State afterwards:
> 4: eth0: <BROADCAST,MULTICAST,UP> mtu 1500 qdisc pfifo_fast qlen 1000
> inet 10.0.0.2/24 scope global eth0
> inet 10.0.0.3/24 scope global secondary eth0
> inet 10.0.0.4/24 scope global secondary eth0
>
> broadcast 10.0.0.0 proto kernel scope link src 10.0.0.2
> local 10.0.0.2 proto kernel scope host src 10.0.0.2
> broadcast 10.0.0.255 proto kernel scope link src 10.0.0.2
>
> Local routes for 10.0.0.3 and 10.0.0.4 have disappeared _without_
> any notification.
>
> I think the correct way to fix this is to prevent the deletion of
> the local routes, not just readding them. _If_ the deletion of them
> is intended, which I doubt, then at least notifications must be
> sent out.
I agree, the routes should ideally not be deleted at all. The missing
notifications appear to be a different bug. Let me have another look ..
^ permalink raw reply
* Re: [PATCH]dgrs - Fixes Warnings when CONFIG_ISA and CONFIG_PCI are not enabled
From: Andrew Morton @ 2005-11-05 2:25 UTC (permalink / raw)
To: Richard Knutsson
Cc: ashutosh.lkml, netdev, davej, acme, linux-net, linux-kernel,
stable
In-Reply-To: <436927CA.3090105@student.ltu.se>
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
Not for this driver. From drivers/net/dgrs.c:
config DGRS
tristate "Digi Intl. RightSwitch SE-X support"
depends on NET_PCI && (PCI || EISA)
> I think you patch is better.
Let's go with Ashutosh's patch then, thanks.
^ permalink raw reply
* Re: [PATCH]dgrs - Fixes Warnings when CONFIG_ISA and CONFIG_PCI are not enabled
From: Andrew Morton @ 2005-11-05 2:30 UTC (permalink / raw)
To: ricknu-0, ashutosh.lkml, netdev, davej, acme, linux-net,
linux-kernel, stable
In-Reply-To: <20051104182537.741be3d9.akpm@osdl.org>
Andrew Morton <akpm@osdl.org> wrote:
>
> Let's go with Ashutosh's patch then, thanks.
(It was wordwrapped. Please fix your email client)
In fact we can de-ifdef things a bit.
diff -puN drivers/net/dgrs.c~dgrs-fixes-warnings-when-config_isa-and-config_pci-are-not-enabled drivers/net/dgrs.c
--- devel/drivers/net/dgrs.c~dgrs-fixes-warnings-when-config_isa-and-config_pci-are-not-enabled 2005-11-04 18:26:59.000000000 -0800
+++ devel-akpm/drivers/net/dgrs.c 2005-11-04 18:29:24.000000000 -0800
@@ -1549,7 +1549,7 @@ MODULE_PARM_DESC(nicmode, "Digi RightSwi
static int __init dgrs_init_module (void)
{
int i;
- int eisacount = 0, pcicount = 0;
+ int cardcount = 0;
/*
* Command line variable overrides
@@ -1591,15 +1591,13 @@ static int __init dgrs_init_module (void
* Find and configure all the cards
*/
#ifdef CONFIG_EISA
- eisacount = eisa_driver_register(&dgrs_eisa_driver);
- if (eisacount < 0)
- return eisacount;
-#endif
-#ifdef CONFIG_PCI
- pcicount = pci_register_driver(&dgrs_pci_driver);
- if (pcicount)
- return pcicount;
+ cardcount = eisa_driver_register(&dgrs_eisa_driver);
+ if (cardcount < 0)
+ return cardcount;
#endif
+ cardcount = pci_register_driver(&dgrs_pci_driver);
+ if (cardcount)
+ return cardcount;
return 0;
}
_
^ permalink raw reply
* Re: [PATCH] kill 8139too kernel thread (sorta)
From: Jeff Garzik @ 2005-11-05 3:47 UTC (permalink / raw)
To: Herbert Xu; +Cc: netdev, linux-kernel
In-Reply-To: <20051031211143.GA6409@gondor.apana.org.au>
[-- Attachment #1: Type: text/plain, Size: 65 bytes --]
Here's a better version, that uses cancel_rearming_...
Jeff
[-- Attachment #2: patch --]
[-- Type: text/plain, Size: 4707 bytes --]
diff --git a/drivers/net/8139too.c b/drivers/net/8139too.c
index 30bee11..120baaa 100644
--- a/drivers/net/8139too.c
+++ b/drivers/net/8139too.c
@@ -586,16 +586,16 @@ struct rtl8139_private {
dma_addr_t tx_bufs_dma;
signed char phys[4]; /* MII device addresses. */
char twistie, twist_row, twist_col; /* Twister tune state. */
- unsigned int default_port:4; /* Last dev->if_port value. */
+ unsigned int default_port : 4; /* Last dev->if_port value. */
+ unsigned int have_thread : 1;
spinlock_t lock;
spinlock_t rx_lock;
chip_t chipset;
- pid_t thr_pid;
- wait_queue_head_t thr_wait;
- struct completion thr_exited;
u32 rx_config;
struct rtl_extra_stats xstats;
- int time_to_die;
+
+ struct work_struct thread;
+
struct mii_if_info mii;
unsigned int regs_len;
unsigned long fifo_copy_timeout;
@@ -620,7 +620,7 @@ static int rtl8139_open (struct net_devi
static int mdio_read (struct net_device *dev, int phy_id, int location);
static void mdio_write (struct net_device *dev, int phy_id, int location,
int val);
-static void rtl8139_start_thread(struct net_device *dev);
+static void rtl8139_start_thread(struct rtl8139_private *tp);
static void rtl8139_tx_timeout (struct net_device *dev);
static void rtl8139_init_ring (struct net_device *dev);
static int rtl8139_start_xmit (struct sk_buff *skb,
@@ -637,6 +637,7 @@ static struct net_device_stats *rtl8139_
static void rtl8139_set_rx_mode (struct net_device *dev);
static void __set_rx_mode (struct net_device *dev);
static void rtl8139_hw_start (struct net_device *dev);
+static void rtl8139_thread (void *_data);
static struct ethtool_ops rtl8139_ethtool_ops;
/* write MMIO register, with flush */
@@ -1007,8 +1008,7 @@ static int __devinit rtl8139_init_one (s
(debug < 0 ? RTL8139_DEF_MSG_ENABLE : ((1 << debug) - 1));
spin_lock_init (&tp->lock);
spin_lock_init (&tp->rx_lock);
- init_waitqueue_head (&tp->thr_wait);
- init_completion (&tp->thr_exited);
+ INIT_WORK(&tp->thread, rtl8139_thread, dev);
tp->mii.dev = dev;
tp->mii.mdio_read = mdio_read;
tp->mii.mdio_write = mdio_write;
@@ -1345,7 +1345,7 @@ static int rtl8139_open (struct net_devi
dev->irq, RTL_R8 (MediaStatus),
tp->mii.full_duplex ? "full" : "half");
- rtl8139_start_thread(dev);
+ rtl8139_start_thread(tp);
return 0;
}
@@ -1594,55 +1594,37 @@ static inline void rtl8139_thread_iter (
RTL_R8 (Config1));
}
-static int rtl8139_thread (void *data)
+static void rtl8139_thread (void *_data)
{
- struct net_device *dev = data;
+ struct net_device *dev = _data;
struct rtl8139_private *tp = netdev_priv(dev);
- unsigned long timeout;
-
- daemonize("%s", dev->name);
- allow_signal(SIGTERM);
-
- while (1) {
- timeout = next_tick;
- do {
- timeout = interruptible_sleep_on_timeout (&tp->thr_wait, timeout);
- /* make swsusp happy with our thread */
- try_to_freeze();
- } while (!signal_pending (current) && (timeout > 0));
-
- if (signal_pending (current)) {
- flush_signals(current);
- }
-
- if (tp->time_to_die)
- break;
- if (rtnl_lock_interruptible ())
- break;
+ if (rtnl_shlock_nowait() == 0) {
rtl8139_thread_iter (dev, tp, tp->mmio_addr);
rtnl_unlock ();
}
- complete_and_exit (&tp->thr_exited, 0);
+ schedule_delayed_work(&tp->thread, next_tick);
}
-static void rtl8139_start_thread(struct net_device *dev)
+static void rtl8139_start_thread(struct rtl8139_private *tp)
{
- struct rtl8139_private *tp = netdev_priv(dev);
-
- tp->thr_pid = -1;
tp->twistie = 0;
- tp->time_to_die = 0;
if (tp->chipset == CH_8139_K)
tp->twistie = 1;
else if (tp->drv_flags & HAS_LNK_CHNG)
return;
- tp->thr_pid = kernel_thread(rtl8139_thread, dev, CLONE_FS|CLONE_FILES);
- if (tp->thr_pid < 0) {
- printk (KERN_WARNING "%s: unable to start kernel thread\n",
- dev->name);
+ tp->have_thread = 1;
+
+ schedule_delayed_work(&tp->thread, next_tick);
+}
+
+static void rtl8139_stop_thread(struct rtl8139_private *tp)
+{
+ if (tp->have_thread) {
+ cancel_rearming_delayed_work(&tp->thread);
+ tp->have_thread = 0;
}
}
@@ -2224,22 +2206,12 @@ static int rtl8139_close (struct net_dev
{
struct rtl8139_private *tp = netdev_priv(dev);
void __iomem *ioaddr = tp->mmio_addr;
- int ret = 0;
unsigned long flags;
netif_stop_queue (dev);
- if (tp->thr_pid >= 0) {
- tp->time_to_die = 1;
- wmb();
- ret = kill_proc (tp->thr_pid, SIGTERM, 1);
- if (ret) {
- printk (KERN_ERR "%s: unable to signal thread\n", dev->name);
- return ret;
- }
- wait_for_completion (&tp->thr_exited);
- }
-
+ rtl8139_stop_thread(tp);
+
if (netif_msg_ifdown(tp))
printk(KERN_DEBUG "%s: Shutting down ethercard, status was 0x%4.4x.\n",
dev->name, RTL_R16 (IntrStatus));
^ permalink raw reply related
* Re: [PATCH] kill 8139too kernel thread (sorta)
From: Herbert Xu @ 2005-11-05 4:20 UTC (permalink / raw)
To: Jeff Garzik; +Cc: netdev, linux-kernel
In-Reply-To: <436C2B47.3030505@pobox.com>
On Fri, Nov 04, 2005 at 10:47:19PM -0500, Jeff Garzik wrote:
> Here's a better version, that uses cancel_rearming_...
Yep it certainly solves the race condition.
> + if (rtnl_shlock_nowait() == 0) {
> rtl8139_thread_iter (dev, tp, tp->mmio_addr);
> rtnl_unlock ();
> }
>
> - complete_and_exit (&tp->thr_exited, 0);
> + schedule_delayed_work(&tp->thread, next_tick);
My only concern is the potential for starvation here should we fail
to obtain the RTNL. Since any local user can hold the RTNL by issuing
rtnetlink requests, it is theoretically possible for the rtl8139 work
to be delayed indefinitely.
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] [IPV4] Fix secondary IP addresses after promotion
From: Patrick McHardy @ 2005-11-05 4:28 UTC (permalink / raw)
To: Thomas Graf
Cc: Brian Pomerantz, netdev, davem, kuznet, pekkas, jmorris, yoshfuji,
kaber, linux-kernel
In-Reply-To: <436C090D.5020201@trash.net>
Patrick McHardy wrote:
> Thomas Graf wrote:
>
>> broadcast 10.0.0.0 proto kernel scope link src 10.0.0.2 local
>> 10.0.0.2 proto kernel scope host src 10.0.0.2 broadcast 10.0.0.255
>> proto kernel scope link src 10.0.0.2
>> Local routes for 10.0.0.3 and 10.0.0.4 have disappeared _without_
>> any notification.
>>
>> I think the correct way to fix this is to prevent the deletion of
>> the local routes, not just readding them. _If_ the deletion of them
>> is intended, which I doubt, then at least notifications must be
>> sent out.
>
> I agree, the routes should ideally not be deleted at all. The missing
> notifications appear to be a different bug. Let me have another look ..
The reason why all routes are deleted is because their prefered
source addresses is the primary address. fn_flush_list should
probably send the missing notifications for the deleted routes.
Changing address promotion to not delete the other routes at all
looks extremly complicated, I think just fixing it to behave
correctly is good enough (which my patch didn't do entirely,
I'll send a new one this weekend).
^ permalink raw reply
* Re: [PATCH]dgrs - Fixes Warnings when CONFIG_ISA and CONFIG_PCI are not enabled
From: Ashutosh Naik @ 2005-11-05 4:51 UTC (permalink / raw)
To: Andrew Morton
Cc: ricknu-0, netdev, davej, acme, linux-net, linux-kernel, stable
In-Reply-To: <20051104183043.27a2229c.akpm@osdl.org>
[-- Attachment #1: Type: text/plain, Size: 1859 bytes --]
Hi Andrew,
On 11/5/05, Andrew Morton <akpm@osdl.org> wrote:
> Andrew Morton <akpm@osdl.org> wrote:
> >
> > Let's go with Ashutosh's patch then, thanks.
>
> (It was wordwrapped. Please fix your email client)
I am attaching the patch.
Signed-off-by: Ashutosh Naik <ashutosh.naik@gmail.com>
> In fact we can de-ifdef things a bit.
>
> diff -puN drivers/net/dgrs.c~dgrs-fixes-warnings-when-config_isa-and-config_pci-are-not-enabled drivers/net/dgrs.c
> --- devel/drivers/net/dgrs.c~dgrs-fixes-warnings-when-config_isa-and-config_pci-are-not-enabled 2005-11-04 18:26:59.000000000 -0800
> +++ devel-akpm/drivers/net/dgrs.c 2005-11-04 18:29:24.000000000 -0800
> @@ -1549,7 +1549,7 @@ MODULE_PARM_DESC(nicmode, "Digi RightSwi
> static int __init dgrs_init_module (void)
> {
> int i;
> - int eisacount = 0, pcicount = 0;
> + int cardcount = 0;
>
> /*
> * Command line variable overrides
> @@ -1591,15 +1591,13 @@ static int __init dgrs_init_module (void
> * Find and configure all the cards
> */
> #ifdef CONFIG_EISA
> - eisacount = eisa_driver_register(&dgrs_eisa_driver);
> - if (eisacount < 0)
> - return eisacount;
> -#endif
> -#ifdef CONFIG_PCI
> - pcicount = pci_register_driver(&dgrs_pci_driver);
> - if (pcicount)
> - return pcicount;
> + cardcount = eisa_driver_register(&dgrs_eisa_driver);
> + if (cardcount < 0)
> + return cardcount;
> #endif
> + cardcount = pci_register_driver(&dgrs_pci_driver);
> + if (cardcount)
> + return cardcount;
> return 0;
> }
>
Signed-off-by: Ashutosh Naik <ashutosh.naik@gmail.com>
Acked. The above patch does the trick too. Any one can be committed.
Cheers
Ashutosh
[-- Attachment #2: dgrs-patch.txt --]
[-- Type: text/plain, Size: 540 bytes --]
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
^ permalink raw reply
* Re: [PATCH] kill 8139too kernel thread (sorta)
From: Jeff Garzik @ 2005-11-05 4:58 UTC (permalink / raw)
To: Herbert Xu; +Cc: netdev, linux-kernel
In-Reply-To: <20051105042008.GA25823@gondor.apana.org.au>
Herbert Xu wrote:
> My only concern is the potential for starvation here should we fail
> to obtain the RTNL. Since any local user can hold the RTNL by issuing
> rtnetlink requests, it is theoretically possible for the rtl8139 work
> to be delayed indefinitely.
Yes, but highly unlikely, for very few users, with the negative effects
negligible.
Jeff
^ permalink raw reply
* Re: [NF+IPsec 4/6]: Make IPsec input processing symetrical to output
From: Herbert Xu @ 2005-11-05 6:30 UTC (permalink / raw)
To: YOSHIFUJI Hideaki / ?$B5HF#1QL@; +Cc: netdev, netfilter-devel, kaber
In-Reply-To: <20051027.235732.01166239.yoshfuji@linux-ipv6.org>
On Thu, Oct 27, 2005 at 11:57:32PM +0900, YOSHIFUJI Hideaki / ?$B5HF#1QL@ wrote:
>
> Well, I really care.
> I strongly believe that we SHOULD NOT mix encrypted
> packets and plain text packets at the same hook.
> e.g. LOCAL_IN is NOT for decrypted plain text packets,
> but for the original encrypted ones.
OK. Would it be workable for you if LOCAL_IN only saw the decrypted
packets without ever seeing the encrypted ones?
I'd like to know where the boundaries are so we can find a compromise
that works for everyone.
Thanks,
--
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply
* Re: [NF+IPsec 4/6]: Make IPsec input processing symetrical to output
From: Patrick McHardy @ 2005-11-05 7:55 UTC (permalink / raw)
To: Herbert Xu; +Cc: netdev, netfilter-devel
In-Reply-To: <20051105063030.GA32385@gondor.apana.org.au>
Herbert Xu wrote:
> On Thu, Oct 27, 2005 at 11:57:32PM +0900, YOSHIFUJI Hideaki / ?$B5HF#1QL@ wrote:
>
>>Well, I really care.
>>I strongly believe that we SHOULD NOT mix encrypted
>>packets and plain text packets at the same hook.
>>e.g. LOCAL_IN is NOT for decrypted plain text packets,
>>but for the original encrypted ones.
>
>
> OK. Would it be workable for you if LOCAL_IN only saw the decrypted
> packets without ever seeing the encrypted ones?
How exactly would that work? I guess we couldn't do NAT with
the encrypted packet anymore?
> I'd like to know where the boundaries are so we can find a compromise
> that works for everyone.
I would prefer something similar to the second set of patches.
Instead of calling netif_rx we could use NF_HOOK and simulate
the relevant parts of the input path for IPv4 and NAT. This
would assure that statistics are still correct and tcpdump is
not affected, which were Yoshifuji's biggest concerns if I
understood correctly.
^ permalink raw reply
* Re: [NF+IPsec 4/6]: Make IPsec input processing symetrical to output
From: YOSHIFUJI Hideaki / 吉藤英明 @ 2005-11-05 8:23 UTC (permalink / raw)
To: herbert, miyazawa, kozakai; +Cc: netdev, netfilter-devel, kaber
In-Reply-To: <20051105063030.GA32385@gondor.apana.org.au>
In article <20051105063030.GA32385@gondor.apana.org.au> (at Sat, 5 Nov 2005 17:30:30 +1100), Herbert Xu <herbert@gondor.apana.org.au> says:
> On Thu, Oct 27, 2005 at 11:57:32PM +0900, YOSHIFUJI Hideaki / ?$B5HF#1QL@ wrote:
> >
> > Well, I really care.
> > I strongly believe that we SHOULD NOT mix encrypted
> > packets and plain text packets at the same hook.
> > e.g. LOCAL_IN is NOT for decrypted plain text packets,
> > but for the original encrypted ones.
>
> OK. Would it be workable for you if LOCAL_IN only saw the decrypted
> packets without ever seeing the encrypted ones?
Miyazawa-san, Kozakai-san and I have discussed on this issue.
The answer is "maybe," but from the point of view of the original context,
it'd be better to let it see encrypted packet.
Miyazawa-san, Kozakai-san, please comment further details.
(I'm about to fly...)
--yoshfuji @ NRT(->YVR)
^ permalink raw reply
* Re: ancient ieee80211/ipw2200 drivers in recent kernel (2.6.14)
From: Harald Welte @ 2005-11-05 8:25 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: debian-kernel, NetDev
In-Reply-To: <20051105002100.GA24128@infradead.org>
[-- Attachment #1: Type: text/plain, Size: 959 bytes --]
On Sat, Nov 05, 2005 at 12:21:00AM +0000, Christoph Hellwig wrote:
> On Fri, Nov 04, 2005 at 12:29:20PM +0900, Horms wrote:
> > do I take that comment to mean that upstream can't update the
> > drivers but Debian can? And if so, do you recommend updating
> > Debian's kernel packages, or putting the updates elsewhere?
>
> Well, we could upstream, but so far no one is annoyed enough to
> overrid the driver maintainer.
This is outrageous.
Do you know any contacts at Intel to whom we could complain? I guess
there would be many people willing to write a nice email about how
impractical (actually, insane) such a policy is?
--
- Harald Welte <laforge@gnumonks.org> http://gnumonks.org/
============================================================================
"Privacy in residential applications is a desirable marketing option."
(ETSI EN 300 175-7 Ch. A6)
[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]
^ permalink raw reply
* Re: [PATCH]dgrs - Fixes Warnings when CONFIG_ISA and CONFIG_PCI are not enabled
From: Richard Knutsson @ 2005-11-05 8:36 UTC (permalink / raw)
To: Andrew Morton
Cc: ashutosh.lkml, netdev, davej, acme, linux-net, linux-kernel,
stable
In-Reply-To: <20051104183043.27a2229c.akpm@osdl.org>
Andrew Morton wrote:
>In fact we can de-ifdef things a bit.
>
>diff -puN drivers/net/dgrs.c~dgrs-fixes-warnings-when-config_isa-and-config_pci-are-not-enabled drivers/net/dgrs.c
>--- devel/drivers/net/dgrs.c~dgrs-fixes-warnings-when-config_isa-and-config_pci-are-not-enabled 2005-11-04 18:26:59.000000000 -0800
>+++ devel-akpm/drivers/net/dgrs.c 2005-11-04 18:29:24.000000000 -0800
>@@ -1549,7 +1549,7 @@ MODULE_PARM_DESC(nicmode, "Digi RightSwi
> static int __init dgrs_init_module (void)
> {
> int i;
>- int eisacount = 0, pcicount = 0;
>+ int cardcount = 0;
>
> /*
> * Command line variable overrides
>@@ -1591,15 +1591,13 @@ static int __init dgrs_init_module (void
> * Find and configure all the cards
> */
> #ifdef CONFIG_EISA
>- eisacount = eisa_driver_register(&dgrs_eisa_driver);
>- if (eisacount < 0)
>- return eisacount;
>-#endif
>-#ifdef CONFIG_PCI
>- pcicount = pci_register_driver(&dgrs_pci_driver);
>- if (pcicount)
>- return pcicount;
>+ cardcount = eisa_driver_register(&dgrs_eisa_driver);
>+ if (cardcount < 0)
>+ return cardcount;
> #endif
>+ cardcount = pci_register_driver(&dgrs_pci_driver);
>+ if (cardcount)
>+ return cardcount;
> return 0;
> }
>
>
I do not know what to think about this one:
* reduce one #ifdef: good
* check for something clearly stated not to: not so good
But as Ashutosh Naik said: Any one can be committed.
/Richard
^ permalink raw reply
* Re: [NF+IPsec 4/6]: Make IPsec input processing symetrical to output
From: Herbert Xu @ 2005-11-05 8:39 UTC (permalink / raw)
To: Patrick McHardy; +Cc: netdev, netfilter-devel
In-Reply-To: <436C6580.6030007@trash.net>
On Sat, Nov 05, 2005 at 08:55:44AM +0100, Patrick McHardy wrote:
>
> >OK. Would it be workable for you if LOCAL_IN only saw the decrypted
> >packets without ever seeing the encrypted ones?
>
> How exactly would that work? I guess we couldn't do NAT with
> the encrypted packet anymore?
I'm presuming that Yoshifuji-san has no objections to applying the
NAT-related hooks twice on IPsec since IPv6 does/will not have NAT.
Given that assumption, we should be able to separate the existing
LOCAL_IN into a read-only (filtering) part and a read-write part.
The latter would be applied unconditionally while the former can
be skipped.
> I would prefer something similar to the second set of patches.
> Instead of calling netif_rx we could use NF_HOOK and simulate
> the relevant parts of the input path for IPv4 and NAT. This
> would assure that statistics are still correct and tcpdump is
> not affected, which were Yoshifuji's biggest concerns if I
> understood correctly.
I don't think netif_rx is the problem here. The question is
how and where do we place the netfilter hooks. Even without
netif_rx the same problem is going to be there.
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]dgrs - Fixes Warnings when CONFIG_ISA and CONFIG_PCI are not enabled
From: Andrew Morton @ 2005-11-05 8:46 UTC (permalink / raw)
To: Richard Knutsson
Cc: ashutosh.lkml, netdev, davej, acme, linux-net, linux-kernel,
stable
In-Reply-To: <436C6F02.90904@student.ltu.se>
Richard Knutsson <ricknu-0@student.ltu.se> wrote:
>
> > */
> > #ifdef CONFIG_EISA
> >- eisacount = eisa_driver_register(&dgrs_eisa_driver);
> >- if (eisacount < 0)
> >- return eisacount;
> >-#endif
> >-#ifdef CONFIG_PCI
> >- pcicount = pci_register_driver(&dgrs_pci_driver);
> >- if (pcicount)
> >- return pcicount;
> >+ cardcount = eisa_driver_register(&dgrs_eisa_driver);
> >+ if (cardcount < 0)
> >+ return cardcount;
> > #endif
> >+ cardcount = pci_register_driver(&dgrs_pci_driver);
> >+ if (cardcount)
> >+ return cardcount;
> > return 0;
> > }
> >
> >
> I do not know what to think about this one:
> * reduce one #ifdef: good
> * check for something clearly stated not to: not so good
Well a nicer fix would be to provide a stub implementation of
eisa_driver_register() if !CONFIG_EISA, just like pci_register_driver().
Then all the ifdefs go away and the compiler removes all the code for us,
after checking that we typed it correctly.
^ permalink raw reply
* Re: [NF+IPsec 4/6]: Make IPsec input processing symetrical to output
From: Patrick McHardy @ 2005-11-05 8:58 UTC (permalink / raw)
To: Herbert Xu; +Cc: netdev, netfilter-devel
In-Reply-To: <20051105083955.GA30293@gondor.apana.org.au>
Herbert Xu wrote:
> On Sat, Nov 05, 2005 at 08:55:44AM +0100, Patrick McHardy wrote:
>
>>>OK. Would it be workable for you if LOCAL_IN only saw the decrypted
>>>packets without ever seeing the encrypted ones?
>>
>>How exactly would that work? I guess we couldn't do NAT with
>>the encrypted packet anymore?
>
> I'm presuming that Yoshifuji-san has no objections to applying the
> NAT-related hooks twice on IPsec since IPv6 does/will not have NAT.
>
> Given that assumption, we should be able to separate the existing
> LOCAL_IN into a read-only (filtering) part and a read-write part.
> The latter would be applied unconditionally while the former can
> be skipped.
So far I don't see why we shouldn't just the LOCAL_IN hook as it
is and also haven't heard any reason against it. I'm fine with
not using netif_rx, but I don't see why we should complicate
things by introducing special semantics for decapsulated transport
mode packets.
>>I would prefer something similar to the second set of patches.
>>Instead of calling netif_rx we could use NF_HOOK and simulate
>>the relevant parts of the input path for IPv4 and NAT. This
>>would assure that statistics are still correct and tcpdump is
>>not affected, which were Yoshifuji's biggest concerns if I
>>understood correctly.
>
> I don't think netif_rx is the problem here. The question is
> how and where do we place the netfilter hooks. Even without
> netif_rx the same problem is going to be there.
IMO the view for netfilter should be as if we called netif_rx,
so on the input path decapsulated packets from the innermost
transport mode SA should go through PRE_ROUTING->LOCAL_IN
or possibly FORWARD in case of NAT.
^ permalink raw reply
* Re: [NF+IPsec 4/6]: Make IPsec input processing symetrical to output
From: Herbert Xu @ 2005-11-05 9:09 UTC (permalink / raw)
To: Patrick McHardy; +Cc: netdev, netfilter-devel
In-Reply-To: <436C7430.5030707@trash.net>
On Sat, Nov 05, 2005 at 09:58:24AM +0100, Patrick McHardy wrote:
>
> So far I don't see why we shouldn't just the LOCAL_IN hook as it
> is and also haven't heard any reason against it. I'm fine with
> not using netif_rx, but I don't see why we should complicate
> things by introducing special semantics for decapsulated transport
> mode packets.
The fewer changes we have to make the happier I am :)
> IMO the view for netfilter should be as if we called netif_rx,
> so on the input path decapsulated packets from the innermost
> transport mode SA should go through PRE_ROUTING->LOCAL_IN
> or possibly FORWARD in case of NAT.
You mean we simply skip the pre-decapsulation LOCAL_IN step
and the post-encapsulation LOCAL_OUT step? That sounds great
to me.
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: [NF+IPsec 4/6]: Make IPsec input processing symetrical to output
From: Patrick McHardy @ 2005-11-05 9:19 UTC (permalink / raw)
To: Herbert Xu; +Cc: netdev, netfilter-devel
In-Reply-To: <20051105090904.GA30733@gondor.apana.org.au>
Herbert Xu wrote:
> On Sat, Nov 05, 2005 at 09:58:24AM +0100, Patrick McHardy wrote:
>
>>IMO the view for netfilter should be as if we called netif_rx,
>>so on the input path decapsulated packets from the innermost
>>transport mode SA should go through PRE_ROUTING->LOCAL_IN
>>or possibly FORWARD in case of NAT.
>
> You mean we simply skip the pre-decapsulation LOCAL_IN step
> and the post-encapsulation LOCAL_OUT step? That sounds great
> to me.
No, that won't be possible if we have more than one SA and
would also make DNAT in LOCAL_OUT on the encapsulated packet
impossible.
What I propose is to keep tunnel mode handling as it is, so
for each tunnel mode SA we hit PRE_ROUTING and LOCAL_IN in
the normal packet path. If the final SA is a transport mode
SA, we don't call netif_rx as in my first patchset, but pass
the packet through a new PRE_ROUTING hook in xfrm{4,6}_input
and LOCAL_IN afterwards. The packet won't be processed a second
time by the stack, just the netfilter hooks will be called.
NAT be will be handled manually for IPv4 by doing a new route
lookup and calling dst_input if NAT took place.
^ permalink raw reply
* Re: [NF+IPsec 4/6]: Make IPsec input processing symetrical to output
From: Herbert Xu @ 2005-11-05 9:38 UTC (permalink / raw)
To: Patrick McHardy; +Cc: netdev, netfilter-devel
In-Reply-To: <436C7937.9070901@trash.net>
On Sat, Nov 05, 2005 at 10:19:51AM +0100, Patrick McHardy wrote:
>
> What I propose is to keep tunnel mode handling as it is, so
> for each tunnel mode SA we hit PRE_ROUTING and LOCAL_IN in
> the normal packet path. If the final SA is a transport mode
> SA, we don't call netif_rx as in my first patchset, but pass
> the packet through a new PRE_ROUTING hook in xfrm{4,6}_input
> and LOCAL_IN afterwards. The packet won't be processed a second
> time by the stack, just the netfilter hooks will be called.
> NAT be will be handled manually for IPv4 by doing a new route
> lookup and calling dst_input if NAT took place.
In other words LOCAL_IN will still see the packet twice for
pure transport mode packets? That's going to be a problem for
me for the reasons that I outlined earlier:
<20051011131838.GA4934@gondor.apana.org.au>
Also, I thought Yoshifuji-san's objection is not just about
transport mode packets passing through netif_rx twice, but
passing through netfilter twice as well?
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: [NF+IPsec 4/6]: Make IPsec input processing symetrical to output
From: Patrick McHardy @ 2005-11-05 9:55 UTC (permalink / raw)
To: Herbert Xu; +Cc: netdev, netfilter-devel
In-Reply-To: <20051105093821.GA30966@gondor.apana.org.au>
Herbert Xu wrote:
> On Sat, Nov 05, 2005 at 10:19:51AM +0100, Patrick McHardy wrote:
>
>>What I propose is to keep tunnel mode handling as it is, so
>>for each tunnel mode SA we hit PRE_ROUTING and LOCAL_IN in
>>the normal packet path. If the final SA is a transport mode
>>SA, we don't call netif_rx as in my first patchset, but pass
>>the packet through a new PRE_ROUTING hook in xfrm{4,6}_input
>>and LOCAL_IN afterwards. The packet won't be processed a second
>>time by the stack, just the netfilter hooks will be called.
>>NAT be will be handled manually for IPv4 by doing a new route
>>lookup and calling dst_input if NAT took place.
>
>
> In other words LOCAL_IN will still see the packet twice for
> pure transport mode packets? That's going to be a problem for
> me for the reasons that I outlined earlier:
>
> <20051011131838.GA4934@gondor.apana.org.au>
Well, once encapsulated and once decapsulated.
What I propose is actually exactly what you suggested in that mail:
> Would it be workable to try something like this? We invoke netfilter
> after each tunnel mode transform as we do now. In addition to that,
> we invoke netfilter at the very end of IPsec processing, that is,
> just before the point where the original xfrm*_rcv_encap would have
> returned.
In my last patchset I did it by calling netif_rx at that point,
now I want to add new hooks.
> Also, I thought Yoshifuji-san's objection is not just about
> transport mode packets passing through netif_rx twice, but
> passing through netfilter twice as well?
I think so, but he didn't mention a reason why he objects to it.
I also don't think it can be done otherwise while still keeping
netfilter "just working" for all cases, which IMO is highly
desirable.
^ 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