* Re: [RFC][PATCH] identify in_dev_get rcu read-side critical sections
From: Herbert Xu @ 2005-10-01 1:13 UTC (permalink / raw)
To: Suzanne Wood; +Cc: paulmck, Robert.Olsson, davem, linux-kernel, netdev, walpole
In-Reply-To: <200509300106.j8U16obP021064@rastaban.cs.pdx.edu>
[-- Attachment #1: Type: text/plain, Size: 987 bytes --]
On Thu, Sep 29, 2005 at 06:06:50PM -0700, Suzanne Wood wrote:
>
> Are there three cases then? RCU protection with refcnt, RCU without refcnt,
> and the bare cast of the dereference?
Correct.
The following patch renames __in_dev_get() to __in_dev_get_rtnl() and
introduces __in_dev_get_rcu() to cover the second case.
1) RCU with refcnt should use in_dev_get().
2) RCU without refcnt should use __in_dev_get_rcu().
3) All others must hold RTNL and use __in_dev_get_rtnl().
There is one exception in net/ipv4/route.c which is in fact a pre-existing
race condition. I've marked it as such so that we remember to fix it.
This patch is based on suggestions and prior work by Suzanne Wood and
Paul McKenney.
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
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
[-- Attachment #2: p --]
[-- Type: text/plain, Size: 17044 bytes --]
diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -2776,7 +2776,7 @@ static u32 bond_glean_dev_ip(struct net_
return 0;
rcu_read_lock();
- idev = __in_dev_get(dev);
+ idev = __in_dev_get_rcu(dev);
if (!idev)
goto out;
diff --git a/drivers/net/wan/sdlamain.c b/drivers/net/wan/sdlamain.c
--- a/drivers/net/wan/sdlamain.c
+++ b/drivers/net/wan/sdlamain.c
@@ -57,6 +57,7 @@
#include <linux/ioport.h> /* request_region(), release_region() */
#include <linux/wanrouter.h> /* WAN router definitions */
#include <linux/wanpipe.h> /* WANPIPE common user API definitions */
+#include <linux/rcupdate.h>
#include <linux/in.h>
#include <asm/io.h> /* phys_to_virt() */
@@ -1268,37 +1269,41 @@ unsigned long get_ip_address(struct net_
struct in_ifaddr *ifaddr;
struct in_device *in_dev;
+ unsigned long addr = 0;
- if ((in_dev = __in_dev_get(dev)) == NULL){
- return 0;
+ rcu_read_lock();
+ if ((in_dev = __in_dev_get_rcu(dev)) == NULL){
+ goto out;
}
if ((ifaddr = in_dev->ifa_list)== NULL ){
- return 0;
+ goto out;
}
switch (option){
case WAN_LOCAL_IP:
- return ifaddr->ifa_local;
+ addr = ifaddr->ifa_local;
break;
case WAN_POINTOPOINT_IP:
- return ifaddr->ifa_address;
+ addr = ifaddr->ifa_address;
break;
case WAN_NETMASK_IP:
- return ifaddr->ifa_mask;
+ addr = ifaddr->ifa_mask;
break;
case WAN_BROADCAST_IP:
- return ifaddr->ifa_broadcast;
+ addr = ifaddr->ifa_broadcast;
break;
default:
- return 0;
+ break;
}
- return 0;
+out:
+ rcu_read_unlock();
+ return addr;
}
void add_gateway(sdla_t *card, struct net_device *dev)
diff --git a/drivers/net/wan/syncppp.c b/drivers/net/wan/syncppp.c
--- a/drivers/net/wan/syncppp.c
+++ b/drivers/net/wan/syncppp.c
@@ -769,7 +769,7 @@ static void sppp_cisco_input (struct spp
u32 addr = 0, mask = ~0; /* FIXME: is the mask correct? */
#ifdef CONFIG_INET
rcu_read_lock();
- if ((in_dev = __in_dev_get(dev)) != NULL)
+ if ((in_dev = __in_dev_get_rcu(dev)) != NULL)
{
for (ifa=in_dev->ifa_list; ifa != NULL;
ifa=ifa->ifa_next) {
diff --git a/drivers/net/wireless/strip.c b/drivers/net/wireless/strip.c
--- a/drivers/net/wireless/strip.c
+++ b/drivers/net/wireless/strip.c
@@ -1352,7 +1352,7 @@ static unsigned char *strip_make_packet(
struct in_device *in_dev;
rcu_read_lock();
- in_dev = __in_dev_get(strip_info->dev);
+ in_dev = __in_dev_get_rcu(strip_info->dev);
if (in_dev == NULL) {
rcu_read_unlock();
return NULL;
@@ -1508,7 +1508,7 @@ static void strip_send(struct strip *str
brd = addr = 0;
rcu_read_lock();
- in_dev = __in_dev_get(strip_info->dev);
+ in_dev = __in_dev_get_rcu(strip_info->dev);
if (in_dev) {
if (in_dev->ifa_list) {
brd = in_dev->ifa_list->ifa_broadcast;
diff --git a/drivers/parisc/led.c b/drivers/parisc/led.c
--- a/drivers/parisc/led.c
+++ b/drivers/parisc/led.c
@@ -37,6 +37,7 @@
#include <linux/proc_fs.h>
#include <linux/ctype.h>
#include <linux/blkdev.h>
+#include <linux/rcupdate.h>
#include <asm/io.h>
#include <asm/processor.h>
#include <asm/hardware.h>
@@ -358,9 +359,10 @@ static __inline__ int led_get_net_activi
/* we are running as tasklet, so locking dev_base
* for reading should be OK */
read_lock(&dev_base_lock);
+ rcu_read_lock();
for (dev = dev_base; dev; dev = dev->next) {
struct net_device_stats *stats;
- struct in_device *in_dev = __in_dev_get(dev);
+ struct in_device *in_dev = __in_dev_get_rcu(dev);
if (!in_dev || !in_dev->ifa_list)
continue;
if (LOOPBACK(in_dev->ifa_list->ifa_local))
@@ -371,6 +373,7 @@ static __inline__ int led_get_net_activi
rx_total += stats->rx_packets;
tx_total += stats->tx_packets;
}
+ rcu_read_unlock();
read_unlock(&dev_base_lock);
retval = 0;
diff --git a/drivers/s390/net/qeth_main.c b/drivers/s390/net/qeth_main.c
--- a/drivers/s390/net/qeth_main.c
+++ b/drivers/s390/net/qeth_main.c
@@ -5200,7 +5200,7 @@ qeth_free_vlan_addresses4(struct qeth_ca
if (!card->vlangrp)
return;
rcu_read_lock();
- in_dev = __in_dev_get(card->vlangrp->vlan_devices[vid]);
+ in_dev = __in_dev_get_rcu(card->vlangrp->vlan_devices[vid]);
if (!in_dev)
goto out;
for (ifa = in_dev->ifa_list; ifa; ifa = ifa->ifa_next) {
@@ -7725,7 +7725,7 @@ qeth_arp_constructor(struct neighbour *n
goto out;
rcu_read_lock();
- in_dev = rcu_dereference(__in_dev_get(dev));
+ in_dev = __in_dev_get_rcu(dev);
if (in_dev == NULL) {
rcu_read_unlock();
return -EINVAL;
diff --git a/include/linux/inetdevice.h b/include/linux/inetdevice.h
--- a/include/linux/inetdevice.h
+++ b/include/linux/inetdevice.h
@@ -142,13 +142,21 @@ static __inline__ int bad_mask(u32 mask,
#define endfor_ifa(in_dev) }
+static inline struct in_device *__in_dev_get_rcu(const struct net_device *dev)
+{
+ struct in_device *in_dev = dev->ip_ptr;
+ if (in_dev)
+ in_dev = rcu_dereference(in_dev);
+ return in_dev;
+}
+
static __inline__ struct in_device *
in_dev_get(const struct net_device *dev)
{
struct in_device *in_dev;
rcu_read_lock();
- in_dev = dev->ip_ptr;
+ in_dev = __in_dev_get_rcu(dev);
if (in_dev)
atomic_inc(&in_dev->refcnt);
rcu_read_unlock();
@@ -156,7 +164,7 @@ in_dev_get(const struct net_device *dev)
}
static __inline__ struct in_device *
-__in_dev_get(const struct net_device *dev)
+__in_dev_get_rtnl(const struct net_device *dev)
{
return (struct in_device*)dev->ip_ptr;
}
diff --git a/net/atm/clip.c b/net/atm/clip.c
--- a/net/atm/clip.c
+++ b/net/atm/clip.c
@@ -310,7 +310,7 @@ static int clip_constructor(struct neigh
if (neigh->type != RTN_UNICAST) return -EINVAL;
rcu_read_lock();
- in_dev = rcu_dereference(__in_dev_get(dev));
+ in_dev = __in_dev_get_rcu(dev);
if (!in_dev) {
rcu_read_unlock();
return -EINVAL;
diff --git a/net/core/netpoll.c b/net/core/netpoll.c
--- a/net/core/netpoll.c
+++ b/net/core/netpoll.c
@@ -703,7 +703,7 @@ int netpoll_setup(struct netpoll *np)
if (!np->local_ip) {
rcu_read_lock();
- in_dev = __in_dev_get(ndev);
+ in_dev = __in_dev_get_rcu(ndev);
if (!in_dev || !in_dev->ifa_list) {
rcu_read_unlock();
diff --git a/net/core/pktgen.c b/net/core/pktgen.c
--- a/net/core/pktgen.c
+++ b/net/core/pktgen.c
@@ -1667,7 +1667,7 @@ static void pktgen_setup_inject(struct p
struct in_device *in_dev;
rcu_read_lock();
- in_dev = __in_dev_get(pkt_dev->odev);
+ in_dev = __in_dev_get_rcu(pkt_dev->odev);
if (in_dev) {
if (in_dev->ifa_list) {
pkt_dev->saddr_min = in_dev->ifa_list->ifa_address;
diff --git a/net/econet/af_econet.c b/net/econet/af_econet.c
--- a/net/econet/af_econet.c
+++ b/net/econet/af_econet.c
@@ -406,7 +406,7 @@ static int econet_sendmsg(struct kiocb *
unsigned long network = 0;
rcu_read_lock();
- idev = __in_dev_get(dev);
+ idev = __in_dev_get_rcu(dev);
if (idev) {
if (idev->ifa_list)
network = ntohl(idev->ifa_list->ifa_address) &
diff --git a/net/ipv4/arp.c b/net/ipv4/arp.c
--- a/net/ipv4/arp.c
+++ b/net/ipv4/arp.c
@@ -241,7 +241,7 @@ static int arp_constructor(struct neighb
neigh->type = inet_addr_type(addr);
rcu_read_lock();
- in_dev = rcu_dereference(__in_dev_get(dev));
+ in_dev = __in_dev_get_rcu(dev);
if (in_dev == NULL) {
rcu_read_unlock();
return -EINVAL;
@@ -990,8 +990,8 @@ static int arp_req_set(struct arpreq *r,
ipv4_devconf.proxy_arp = 1;
return 0;
}
- if (__in_dev_get(dev)) {
- __in_dev_get(dev)->cnf.proxy_arp = 1;
+ if (__in_dev_get_rtnl(dev)) {
+ __in_dev_get_rtnl(dev)->cnf.proxy_arp = 1;
return 0;
}
return -ENXIO;
@@ -1096,8 +1096,8 @@ static int arp_req_delete(struct arpreq
ipv4_devconf.proxy_arp = 0;
return 0;
}
- if (__in_dev_get(dev)) {
- __in_dev_get(dev)->cnf.proxy_arp = 0;
+ if (__in_dev_get_rtnl(dev)) {
+ __in_dev_get_rtnl(dev)->cnf.proxy_arp = 0;
return 0;
}
return -ENXIO;
diff --git a/net/ipv4/devinet.c b/net/ipv4/devinet.c
--- a/net/ipv4/devinet.c
+++ b/net/ipv4/devinet.c
@@ -351,7 +351,7 @@ static int inet_insert_ifa(struct in_ifa
static int inet_set_ifa(struct net_device *dev, struct in_ifaddr *ifa)
{
- struct in_device *in_dev = __in_dev_get(dev);
+ struct in_device *in_dev = __in_dev_get_rtnl(dev);
ASSERT_RTNL();
@@ -449,7 +449,7 @@ static int inet_rtm_newaddr(struct sk_bu
goto out;
rc = -ENOBUFS;
- if ((in_dev = __in_dev_get(dev)) == NULL) {
+ if ((in_dev = __in_dev_get_rtnl(dev)) == NULL) {
in_dev = inetdev_init(dev);
if (!in_dev)
goto out;
@@ -584,7 +584,7 @@ int devinet_ioctl(unsigned int cmd, void
if (colon)
*colon = ':';
- if ((in_dev = __in_dev_get(dev)) != NULL) {
+ if ((in_dev = __in_dev_get_rtnl(dev)) != NULL) {
if (tryaddrmatch) {
/* Matthias Andree */
/* compare label and address (4.4BSD style) */
@@ -748,7 +748,7 @@ rarok:
static int inet_gifconf(struct net_device *dev, char __user *buf, int len)
{
- struct in_device *in_dev = __in_dev_get(dev);
+ struct in_device *in_dev = __in_dev_get_rtnl(dev);
struct in_ifaddr *ifa;
struct ifreq ifr;
int done = 0;
@@ -791,7 +791,7 @@ u32 inet_select_addr(const struct net_de
struct in_device *in_dev;
rcu_read_lock();
- in_dev = __in_dev_get(dev);
+ in_dev = __in_dev_get_rcu(dev);
if (!in_dev)
goto no_in_dev;
@@ -818,7 +818,7 @@ no_in_dev:
read_lock(&dev_base_lock);
rcu_read_lock();
for (dev = dev_base; dev; dev = dev->next) {
- if ((in_dev = __in_dev_get(dev)) == NULL)
+ if ((in_dev = __in_dev_get_rcu(dev)) == NULL)
continue;
for_primary_ifa(in_dev) {
@@ -887,7 +887,7 @@ u32 inet_confirm_addr(const struct net_d
if (dev) {
rcu_read_lock();
- if ((in_dev = __in_dev_get(dev)))
+ if ((in_dev = __in_dev_get_rcu(dev)))
addr = confirm_addr_indev(in_dev, dst, local, scope);
rcu_read_unlock();
@@ -897,7 +897,7 @@ u32 inet_confirm_addr(const struct net_d
read_lock(&dev_base_lock);
rcu_read_lock();
for (dev = dev_base; dev; dev = dev->next) {
- if ((in_dev = __in_dev_get(dev))) {
+ if ((in_dev = __in_dev_get_rcu(dev))) {
addr = confirm_addr_indev(in_dev, dst, local, scope);
if (addr)
break;
@@ -957,7 +957,7 @@ static int inetdev_event(struct notifier
void *ptr)
{
struct net_device *dev = ptr;
- struct in_device *in_dev = __in_dev_get(dev);
+ struct in_device *in_dev = __in_dev_get_rtnl(dev);
ASSERT_RTNL();
@@ -1078,7 +1078,7 @@ static int inet_dump_ifaddr(struct sk_bu
if (idx > s_idx)
s_ip_idx = 0;
rcu_read_lock();
- if ((in_dev = __in_dev_get(dev)) == NULL) {
+ if ((in_dev = __in_dev_get_rcu(dev)) == NULL) {
rcu_read_unlock();
continue;
}
@@ -1149,7 +1149,7 @@ void inet_forward_change(void)
for (dev = dev_base; dev; dev = dev->next) {
struct in_device *in_dev;
rcu_read_lock();
- in_dev = __in_dev_get(dev);
+ in_dev = __in_dev_get_rcu(dev);
if (in_dev)
in_dev->cnf.forwarding = on;
rcu_read_unlock();
diff --git a/net/ipv4/fib_frontend.c b/net/ipv4/fib_frontend.c
--- a/net/ipv4/fib_frontend.c
+++ b/net/ipv4/fib_frontend.c
@@ -173,7 +173,7 @@ int fib_validate_source(u32 src, u32 dst
no_addr = rpf = 0;
rcu_read_lock();
- in_dev = __in_dev_get(dev);
+ in_dev = __in_dev_get_rcu(dev);
if (in_dev) {
no_addr = in_dev->ifa_list == NULL;
rpf = IN_DEV_RPFILTER(in_dev);
@@ -607,7 +607,7 @@ static int fib_inetaddr_event(struct not
static int fib_netdev_event(struct notifier_block *this, unsigned long event, void *ptr)
{
struct net_device *dev = ptr;
- struct in_device *in_dev = __in_dev_get(dev);
+ struct in_device *in_dev = __in_dev_get_rtnl(dev);
if (event == NETDEV_UNREGISTER) {
fib_disable_ip(dev, 2);
diff --git a/net/ipv4/fib_semantics.c b/net/ipv4/fib_semantics.c
--- a/net/ipv4/fib_semantics.c
+++ b/net/ipv4/fib_semantics.c
@@ -1087,7 +1087,7 @@ fib_convert_rtentry(int cmd, struct nlms
rta->rta_oif = &dev->ifindex;
if (colon) {
struct in_ifaddr *ifa;
- struct in_device *in_dev = __in_dev_get(dev);
+ struct in_device *in_dev = __in_dev_get_rtnl(dev);
if (!in_dev)
return -ENODEV;
*colon = ':';
@@ -1268,7 +1268,7 @@ int fib_sync_up(struct net_device *dev)
}
if (nh->nh_dev == NULL || !(nh->nh_dev->flags&IFF_UP))
continue;
- if (nh->nh_dev != dev || __in_dev_get(dev) == NULL)
+ if (nh->nh_dev != dev || !__in_dev_get_rtnl(dev))
continue;
alive++;
spin_lock_bh(&fib_multipath_lock);
diff --git a/net/ipv4/igmp.c b/net/ipv4/igmp.c
--- a/net/ipv4/igmp.c
+++ b/net/ipv4/igmp.c
@@ -1323,7 +1323,7 @@ static struct in_device * ip_mc_find_dev
}
if (dev) {
imr->imr_ifindex = dev->ifindex;
- idev = __in_dev_get(dev);
+ idev = __in_dev_get_rtnl(dev);
}
return idev;
}
diff --git a/net/ipv4/ip_gre.c b/net/ipv4/ip_gre.c
--- a/net/ipv4/ip_gre.c
+++ b/net/ipv4/ip_gre.c
@@ -1104,10 +1104,10 @@ static int ipgre_open(struct net_device
return -EADDRNOTAVAIL;
dev = rt->u.dst.dev;
ip_rt_put(rt);
- if (__in_dev_get(dev) == NULL)
+ if (__in_dev_get_rtnl(dev) == NULL)
return -EADDRNOTAVAIL;
t->mlink = dev->ifindex;
- ip_mc_inc_group(__in_dev_get(dev), t->parms.iph.daddr);
+ ip_mc_inc_group(__in_dev_get_rtnl(dev), t->parms.iph.daddr);
}
return 0;
}
diff --git a/net/ipv4/ipmr.c b/net/ipv4/ipmr.c
--- a/net/ipv4/ipmr.c
+++ b/net/ipv4/ipmr.c
@@ -149,7 +149,7 @@ struct net_device *ipmr_new_tunnel(struc
if (err == 0 && (dev = __dev_get_by_name(p.name)) != NULL) {
dev->flags |= IFF_MULTICAST;
- in_dev = __in_dev_get(dev);
+ in_dev = __in_dev_get_rtnl(dev);
if (in_dev == NULL && (in_dev = inetdev_init(dev)) == NULL)
goto failure;
in_dev->cnf.rp_filter = 0;
@@ -278,7 +278,7 @@ static int vif_delete(int vifi)
dev_set_allmulti(dev, -1);
- if ((in_dev = __in_dev_get(dev)) != NULL) {
+ if ((in_dev = __in_dev_get_rtnl(dev)) != NULL) {
in_dev->cnf.mc_forwarding--;
ip_rt_multicast_event(in_dev);
}
@@ -421,7 +421,7 @@ static int vif_add(struct vifctl *vifc,
return -EINVAL;
}
- if ((in_dev = __in_dev_get(dev)) == NULL)
+ if ((in_dev = __in_dev_get_rtnl(dev)) == NULL)
return -EADDRNOTAVAIL;
in_dev->cnf.mc_forwarding++;
dev_set_allmulti(dev, +1);
diff --git a/net/ipv4/netfilter/ip_conntrack_netbios_ns.c b/net/ipv4/netfilter/ip_conntrack_netbios_ns.c
--- a/net/ipv4/netfilter/ip_conntrack_netbios_ns.c
+++ b/net/ipv4/netfilter/ip_conntrack_netbios_ns.c
@@ -58,7 +58,7 @@ static int help(struct sk_buff **pskb,
goto out;
rcu_read_lock();
- in_dev = __in_dev_get(rt->u.dst.dev);
+ in_dev = __in_dev_get_rcu(rt->u.dst.dev);
if (in_dev != NULL) {
for_primary_ifa(in_dev) {
if (ifa->ifa_broadcast == iph->daddr) {
diff --git a/net/ipv4/netfilter/ipt_REDIRECT.c b/net/ipv4/netfilter/ipt_REDIRECT.c
--- a/net/ipv4/netfilter/ipt_REDIRECT.c
+++ b/net/ipv4/netfilter/ipt_REDIRECT.c
@@ -93,7 +93,7 @@ redirect_target(struct sk_buff **pskb,
newdst = 0;
rcu_read_lock();
- indev = __in_dev_get((*pskb)->dev);
+ indev = __in_dev_get_rcu((*pskb)->dev);
if (indev && (ifa = indev->ifa_list))
newdst = ifa->ifa_local;
rcu_read_unlock();
diff --git a/net/ipv4/route.c b/net/ipv4/route.c
--- a/net/ipv4/route.c
+++ b/net/ipv4/route.c
@@ -2128,7 +2128,7 @@ int ip_route_input(struct sk_buff *skb,
struct in_device *in_dev;
rcu_read_lock();
- if ((in_dev = __in_dev_get(dev)) != NULL) {
+ if ((in_dev = __in_dev_get_rcu(dev)) != NULL) {
int our = ip_check_mc(in_dev, daddr, saddr,
skb->nh.iph->protocol);
if (our
@@ -2443,7 +2443,9 @@ static int ip_route_output_slow(struct r
err = -ENODEV;
if (dev_out == NULL)
goto out;
- if (__in_dev_get(dev_out) == NULL) {
+
+ /* RACE: Check return value of inet_select_addr instead. */
+ if (__in_dev_get_rtnl(dev_out) == NULL) {
dev_put(dev_out);
goto out; /* Wrong error code */
}
diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -1806,7 +1806,7 @@ static void sit_add_v4_addrs(struct inet
}
for (dev = dev_base; dev != NULL; dev = dev->next) {
- struct in_device * in_dev = __in_dev_get(dev);
+ struct in_device * in_dev = __in_dev_get_rtnl(dev);
if (in_dev && (dev->flags & IFF_UP)) {
struct in_ifaddr * ifa;
diff --git a/net/irda/irlan/irlan_eth.c b/net/irda/irlan/irlan_eth.c
--- a/net/irda/irlan/irlan_eth.c
+++ b/net/irda/irlan/irlan_eth.c
@@ -310,7 +310,7 @@ void irlan_eth_send_gratuitous_arp(struc
#ifdef CONFIG_INET
IRDA_DEBUG(4, "IrLAN: Sending gratuitous ARP\n");
rcu_read_lock();
- in_dev = __in_dev_get(dev);
+ in_dev = __in_dev_get_rcu(dev);
if (in_dev == NULL)
goto out;
if (in_dev->ifa_list)
diff --git a/net/sctp/protocol.c b/net/sctp/protocol.c
--- a/net/sctp/protocol.c
+++ b/net/sctp/protocol.c
@@ -147,7 +147,7 @@ static void sctp_v4_copy_addrlist(struct
struct sctp_sockaddr_entry *addr;
rcu_read_lock();
- if ((in_dev = __in_dev_get(dev)) == NULL) {
+ if ((in_dev = __in_dev_get_rcu(dev)) == NULL) {
rcu_read_unlock();
return;
}
^ permalink raw reply
* Re: Starfire (Adaptec) kernel 2.6.13+ panics on AMD64 NFS server
From: Herbert Xu @ 2005-09-30 22:39 UTC (permalink / raw)
To: Hendrik Visage
Cc: Andrew Morton, linux-net, linux-kernel, ionut, Jeff Garzik,
netdev
In-Reply-To: <d93f04c70509301310y4bde1189wbcaef40124af6766@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 843 bytes --]
On Fri, Sep 30, 2005 at 08:10:59PM +0000, Hendrik Visage wrote:
>
> Anycase, here is a non-PREEMPT traceback. What makes this one
> interesting, is that
> in the preempt case, I had to push the NFS output to get the panic, but the
> non-preempt case attached, sorta just happened, ie. when the clients
> just checked on the server's status :(
You must never call skb_checksum_help unless the packet is meant to
be checksummed by the hardware. So starfire is the guilty party here.
This patch makes it do the check and also check for errors from
skb_checksum_help.
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
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
[-- Attachment #2: p --]
[-- Type: text/plain, Size: 654 bytes --]
diff --git a/drivers/net/starfire.c b/drivers/net/starfire.c
--- a/drivers/net/starfire.c
+++ b/drivers/net/starfire.c
@@ -1333,7 +1333,7 @@ static int start_tx(struct sk_buff *skb,
}
#if defined(ZEROCOPY) && defined(HAS_BROKEN_FIRMWARE)
- {
+ if (skb->ip_summed == CHECKSUM_HW) {
int has_bad_length = 0;
if (skb_first_frag_len(skb) == 1)
@@ -1346,8 +1346,10 @@ static int start_tx(struct sk_buff *skb,
}
}
- if (has_bad_length)
- skb_checksum_help(skb, 0);
+ if (has_bad_length && unlikely(skb_checksum_help(skb, 0))) {
+ dev_kfree_skb(skb);
+ return NETDEV_TX_OK;
+ }
}
#endif /* ZEROCOPY && HAS_BROKEN_FIRMWARE */
^ permalink raw reply
* [ANNOUNCE] iproute2 version (050929)
From: Stephen Hemminger @ 2005-09-30 22:04 UTC (permalink / raw)
To: netdev, lartc
There is an new minor update to iproute2 utilities available:
http://developer.osdl.org/dev/iproute2/download/iproute2-050929.tar.gz
Also, this (and future) releases are now signed, for key info read:
http://developer.osdl.org/dev/iproute2/signature.html
[Stephen Hemminger]
Fix uninitialized memory and leaks
Add -batch option to ip.
Update to 2.6.14 headers
[Arnaldo]
Integrate support for DCCP into 'ss'
[Mike Frysinger]
Fix build issues with netem tables (parallel make and HOSTCC)
[Eric Dumazet]
Fix lnstat : First column should not be summed
--
Stephen Hemminger <shemminger@osdl.org>
OSDL http://developer.osdl.org/~shemminger
^ permalink raw reply
* Re: ipvs_syncmaster brings cpu to 100%
From: Luca Maranzano @ 2005-09-30 15:59 UTC (permalink / raw)
To: Nishanth Aravamudan
Cc: Dave Miller, Wensong Zhang, Julian Anastasov, netdev, horms
In-Reply-To: <20050928132639.GA5791@us.ibm.com>
First of all thank you all for your precious support! :-)
The two machines on which I discovered the problem are now in
production and I cannot for the moment make tests, but I hope to have
some other hardware to try in the next week.
I'll let you know ASAP.
Thanks,
Luca
>
> Yes, the information in that thread is the same as what Luca said. It's
> a load average problem, not a CPU utilisation problem (those threads are
> sleeping!) If Luca could test the msleep_interruptible() version of the
> patch and it works (like I said, performance should not change, but the
> load average will drop to by 2), then I will ACK the patch for mainline
> acceptance.
>
> Thanks,
> Nish
>
^ permalink raw reply
* Re: [ANNOUNCE] Release of nf-HiPAC 0.9.0
From: Harald Welte @ 2005-09-30 12:33 UTC (permalink / raw)
To: Michael Bellion; +Cc: linux-kernel, linux-net, netdev
In-Reply-To: <200509260445.46740.mbellion@hipac.org>
[-- Attachment #1: Type: text/plain, Size: 798 bytes --]
On Mon, Sep 26, 2005 at 04:45:46AM +0200, Michael Bellion wrote:
> Hi
>
> I am happy to announce the release of nf-HiPAC version 0.9.0
I'm happy to hear this, especially in the advent of the netfilter
develpoer workshop next week, and after a very long period of silence
from the nf-hipac project.
I'll make sure to have read through your 0.9.0 version source code until
then, to be able to give some feedback asap.
Looking forward to talking to you about it next week!
--
- 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
* Funny timestamps (Was: Re: rwlock recursion on CPU#0, netfilter related?)
From: Pekka Pietikainen @ 2005-09-30 6:49 UTC (permalink / raw)
To: netdev
In-Reply-To: <20050929120538.GT4168@sunbeam.de.gnumonks.org>
On Thu, Sep 29, 2005 at 02:05:38PM +0200, Harald Welte wrote:
> > This one is still around, so it's a different bug. Looks like it's a 64-bit
> > issue, a 32-bit ping gives realistic ping times. tcpdump timestamps are also
> > affected, they're completely off too. So looks like someone broke packet
> > timestamps on 64-bit some time after 2.6.13.
>
> luckily I'm not the core network maintainer ;)
Here's the actual bug report:
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=168166
Ended up being a userspace thing, maybe. But still makes me wonder what
change actually broke things. It must have been something soon after 2.6.13.
And there's still tcpdump, which doesn't seem to go into the problem mode
when I test it now, except that nothing should have changed in the
kernel/tcpdump/libpcap versions. Blah.
^ permalink raw reply
* [PATCH 09/10] [PATCH] tcp: set default congestion control correctly for incoming connections
From: Chris Wright @ 2005-09-30 2:20 UTC (permalink / raw)
To: linux-kernel, stable, David S. Miller
Cc: Justin Forbes, Zwane Mwaikambo, Theodore Ts'o, Randy Dunlap,
Chuck Wolber, torvalds, akpm, alan, Stephen Hemminger, netdev,
Joel Sing, Chris Wright
In-Reply-To: <20050930022016.640197000@localhost.localdomain>
[-- Attachment #1: tcp-set-default-congestion-control-correctly.patch --]
[-- Type: text/plain, Size: 1197 bytes --]
-stable review patch. If anyone has any objections, please let us know.
------------------
Patch from Joel Sing to fix the default congestion control algorithm for incoming connections. If a new congestion control handler is added (via module),
it should become the default for new connections. Instead, the incoming
connections use reno. The cause is incorrect
initialisation causes the tcp_init_congestion_control() function to return
after the initial if test fails.
Signed-off-by: Stephen Hemminger <shemminger@osdl.org>
Acked-by: "David S. Miller" <davem@davemloft.net>
Signed-off-by: Chris Wright <chrisw@osdl.org>
---
net/ipv4/tcp_minisocks.c | 2 +-
1 files changed, 1 insertion(+), 1 deletion(-)
Index: linux-2.6.13.y/net/ipv4/tcp_minisocks.c
===================================================================
--- linux-2.6.13.y.orig/net/ipv4/tcp_minisocks.c
+++ linux-2.6.13.y/net/ipv4/tcp_minisocks.c
@@ -774,7 +774,7 @@ struct sock *tcp_create_openreq_child(st
newtp->frto_counter = 0;
newtp->frto_highmark = 0;
- newtp->ca_ops = &tcp_reno;
+ newtp->ca_ops = &tcp_init_congestion_ops;
tcp_set_ca_state(newtp, TCP_CA_Open);
tcp_init_xmit_timers(newsk);
--
^ permalink raw reply
* Re: [RFC][PATCH] identify in_dev_get rcu read-side critical sections
From: Herbert Xu @ 2005-09-30 1:19 UTC (permalink / raw)
To: Paul E. McKenney
Cc: Suzanne Wood, Robert.Olsson, davem, linux-kernel, netdev, walpole
In-Reply-To: <20050930011603.GT8177@us.ibm.com>
On Thu, Sep 29, 2005 at 06:16:03PM -0700, Paul E. McKenney wrote:
>
> OK, how about this instead?
>
> rcu_read_lock();
> in_dev = dev->ip_ptr;
> if (in_dev) {
> atomic_inc(&rcu_dereference(in_dev)->refcnt);
> }
> rcu_read_unlock();
> return in_dev;
Looks great. Thanks Paul.
--
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: [RFC][PATCH] identify in_dev_get rcu read-side critical sections
From: Paul E. McKenney @ 2005-09-30 1:16 UTC (permalink / raw)
To: Herbert Xu
Cc: Suzanne Wood, Robert.Olsson, davem, linux-kernel, netdev, walpole
In-Reply-To: <20050930010404.GA21429@gondor.apana.org.au>
On Fri, Sep 30, 2005 at 11:04:04AM +1000, Herbert Xu wrote:
> On Thu, Sep 29, 2005 at 05:36:42PM -0700, Paul E. McKenney wrote:
> >
> > > rcu_read_lock();
> > > in_dev = dev->ip_ptr;
> > > if (in_dev) {
> > > in_dev = rcu_dereference(in_dev);
> > > atomic_inc(&in_dev->refcnt);
> > > }
> > > rcu_read_unlock();
> > > return in_dev;
> >
> > How about:
> >
> > rcu_read_lock();
> > in_dev = dev->ip_ptr;
> > if (rcu_dereference(in_dev)) {
> > atomic_inc(&in_dev->refcnt);
> > }
> > rcu_read_unlock();
> > return in_dev;
>
> With this the barrier will taken even when in_dev is NULL.
>
> I agree this isn't such a big deal since it only impacts Alpha and then
> only when in_dev is NULL. But as we already do the branch anyway to
> increment the reference count, we might as well make things a little
> better for Alpha.
OK, how about this instead?
rcu_read_lock();
in_dev = dev->ip_ptr;
if (in_dev) {
atomic_inc(&rcu_dereference(in_dev)->refcnt);
}
rcu_read_unlock();
return in_dev;
Thanx, Paul
^ permalink raw reply
* Re: [RFC][PATCH] identify in_dev_get rcu read-side critical sections
From: Suzanne Wood @ 2005-09-30 1:06 UTC (permalink / raw)
To: paulmck, suzannew
Cc: Robert.Olsson, davem, herbert, linux-kernel, netdev, walpole
In reviewing the 44 kernel uses of __in_dev_get and seeing many
cases in 13 of 20 C code files of insertions of rcu_read_lock with
and without the rcu_dereference that is indicated, so it does appear
often to be programmer intent.
Of the programs using __in_dev_get that don't include rcu, devinet.c
and igmp.c use an rtnl lock. Five other programs that use __in_dev_get
without rcu have rtnl locking in the program source code, but I need
to actually look further into the call tree to say more.
Are there three cases then? RCU protection with refcnt, RCU without refcnt,
and the bare cast of the dereference?
Thank you very much for getting it back on track.
> From paulmck@us.ibm.com Thu Sep 29 17:23:18 2005
> Is there any case where __in_dev_get() might be called without
> needing to be wrapped with rcu_dereference()? If so, then I
> agree (FWIW, given my meagre knowledge of Linux networking).
> If all __in_dev_get() invocations need to be wrapped in
> rcu_dereference(), then it seems to me that there would be
> motivation to bury rcu_dereference() in __in_dev_get().
> > > Some callers of __in_dev_get() don't need rcu_dereference at all
> > > because they're protected by the rtnl.
> >
> > > BTW, could you please move the rcu_dereference in in_dev_get()
> > > into the if clause? The barrier is not needed when ip_ptr is
> > > NULL.
> >
> > The trouble with that may be that there are three events, the
> > dereference, the assignment, and the conditional test. The
> > rcu_dereference() is meant to assure deferred destruction
> > throughout.
> One only needs an rcu_dereference() once on the data-flow path from
> fetching the RCU-protected pointer to dereferencing that pointer.
> If the pointer is NULL, there is no way you can dereference it,
> so, technically, Herbert is quite correct.
> However, rcu_dereference() only generates a memory barrier on DEC
> Alpha, so there is normally no penalty for using it in the NULL-pointer
> case. So, when using rcu_dereference() unconditionally simplifies
> the code, it may make sense to "just do it".
> Thanx, Paul
^ permalink raw reply
* Re: [RFC][PATCH] identify in_dev_get rcu read-side critical sections
From: Herbert Xu @ 2005-09-30 1:04 UTC (permalink / raw)
To: Paul E. McKenney
Cc: Suzanne Wood, Robert.Olsson, davem, linux-kernel, netdev, walpole
In-Reply-To: <20050930003642.GQ8177@us.ibm.com>
On Thu, Sep 29, 2005 at 05:36:42PM -0700, Paul E. McKenney wrote:
>
> > rcu_read_lock();
> > in_dev = dev->ip_ptr;
> > if (in_dev) {
> > in_dev = rcu_dereference(in_dev);
> > atomic_inc(&in_dev->refcnt);
> > }
> > rcu_read_unlock();
> > return in_dev;
>
> How about:
>
> rcu_read_lock();
> in_dev = dev->ip_ptr;
> if (rcu_dereference(in_dev)) {
> atomic_inc(&in_dev->refcnt);
> }
> rcu_read_unlock();
> return in_dev;
With this the barrier will taken even when in_dev is NULL.
I agree this isn't such a big deal since it only impacts Alpha and then
only when in_dev is NULL. But as we already do the branch anyway to
increment the reference count, we might as well make things a little
better for Alpha.
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: [RFC][PATCH] identify in_dev_get rcu read-side critical sections
From: Paul E. McKenney @ 2005-09-30 0:36 UTC (permalink / raw)
To: Herbert Xu
Cc: Suzanne Wood, Robert.Olsson, davem, linux-kernel, netdev, walpole
In-Reply-To: <20050930002719.GC21062@gondor.apana.org.au>
On Fri, Sep 30, 2005 at 10:27:19AM +1000, Herbert Xu wrote:
> On Thu, Sep 29, 2005 at 05:23:46PM -0700, Paul E. McKenney wrote:
> >
> > Is there any case where __in_dev_get() might be called without
> > needing to be wrapped with rcu_dereference()? If so, then I
> > agree (FWIW, given my meagre knowledge of Linux networking).
>
> Yes. All paths that call __in_dev_get() under the rtnl do not
> need rcu_dereference (or any RCU at all) since the rtnl prevents
> any ip_ptr modification from occuring.
>
> > However, rcu_dereference() only generates a memory barrier on DEC
> > Alpha, so there is normally no penalty for using it in the NULL-pointer
> > case. So, when using rcu_dereference() unconditionally simplifies
> > the code, it may make sense to "just do it".
>
> Here is what the code would look like:
>
> rcu_read_lock();
> in_dev = dev->ip_ptr;
> if (in_dev) {
> in_dev = rcu_dereference(in_dev);
> atomic_inc(&in_dev->refcnt);
> }
> rcu_read_unlock();
> return in_dev;
How about:
rcu_read_lock();
in_dev = dev->ip_ptr;
if (rcu_dereference(in_dev)) {
atomic_inc(&in_dev->refcnt);
}
rcu_read_unlock();
return in_dev;
Admittedly only saves one line, but...
Thanx, Paul
^ permalink raw reply
* Re: Possible BUG in IPv4 TCP window handling, all recent 2.4.x/2.6.x kernels
From: David S. Miller @ 2005-09-30 0:29 UTC (permalink / raw)
To: kuznet; +Cc: lists, linux-kernel, linux-net, netdev, gautran
In-Reply-To: <20050929151729.GA2158@ms2.inr.ac.ru>
From: Alexey Kuznetsov <kuznet@ms2.inr.ac.ru>
Date: Thu, 29 Sep 2005 19:17:29 +0400
> Good. I think the patch is to be applied to all mainstream kernels.
Done, thanks everyone.
^ permalink raw reply
* Re: [RFC][PATCH] identify in_dev_get rcu read-side critical sections
From: Herbert Xu @ 2005-09-30 0:27 UTC (permalink / raw)
To: Paul E. McKenney
Cc: Suzanne Wood, Robert.Olsson, davem, linux-kernel, netdev, walpole
In-Reply-To: <20050930002346.GP8177@us.ibm.com>
On Thu, Sep 29, 2005 at 05:23:46PM -0700, Paul E. McKenney wrote:
>
> Is there any case where __in_dev_get() might be called without
> needing to be wrapped with rcu_dereference()? If so, then I
> agree (FWIW, given my meagre knowledge of Linux networking).
Yes. All paths that call __in_dev_get() under the rtnl do not
need rcu_dereference (or any RCU at all) since the rtnl prevents
any ip_ptr modification from occuring.
> However, rcu_dereference() only generates a memory barrier on DEC
> Alpha, so there is normally no penalty for using it in the NULL-pointer
> case. So, when using rcu_dereference() unconditionally simplifies
> the code, it may make sense to "just do it".
Here is what the code would look like:
rcu_read_lock();
in_dev = dev->ip_ptr;
if (in_dev) {
in_dev = rcu_dereference(in_dev);
atomic_inc(&in_dev->refcnt);
}
rcu_read_unlock();
return in_dev;
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: [RFC][PATCH] identify in_dev_get rcu read-side critical sections
From: Paul E. McKenney @ 2005-09-30 0:23 UTC (permalink / raw)
To: Suzanne Wood; +Cc: herbert, Robert.Olsson, davem, linux-kernel, netdev, walpole
In-Reply-To: <200509292330.j8TNUSmH019572@rastaban.cs.pdx.edu>
On Thu, Sep 29, 2005 at 04:30:28PM -0700, Suzanne Wood wrote:
> > Date: Fri, 30 Sep 2005 07:28:36 +1000
> > From: Herbert Xu <herbert@gondor.apana.org.au>
>
> > On Thu, Sep 29, 2005 at 09:02:29AM -0700, Suzanne Wood wrote:
> > >
> > > The exchange below suggests that it is equally important
> > > to have the rcu_dereference() in __in_dev_get(), so the
> > > idea of the only difference between in_dev_get and
> > > __in_dev_get being the refcnt may be accepted.
>
> > With __in_dev_get() it's the caller's responsibility to ensure
> > that RCU works correctly. Therefore if any rcu_dereference is
> > needed it should be done by the caller.
>
> This sounds reasonable to me. Does everyone agree?
Is there any case where __in_dev_get() might be called without
needing to be wrapped with rcu_dereference()? If so, then I
agree (FWIW, given my meagre knowledge of Linux networking).
If all __in_dev_get() invocations need to be wrapped in
rcu_dereference(), then it seems to me that there would be
motivation to bury rcu_dereference() in __in_dev_get().
> > Some callers of __in_dev_get() don't need rcu_dereference at all
> > because they're protected by the rtnl.
>
> > BTW, could you please move the rcu_dereference in in_dev_get()
> > into the if clause? The barrier is not needed when ip_ptr is
> > NULL.
>
> The trouble with that may be that there are three events, the
> dereference, the assignment, and the conditional test. The
> rcu_dereference() is meant to assure deferred destruction
> throughout.
One only needs an rcu_dereference() once on the data-flow path from
fetching the RCU-protected pointer to dereferencing that pointer.
If the pointer is NULL, there is no way you can dereference it,
so, technically, Herbert is quite correct.
However, rcu_dereference() only generates a memory barrier on DEC
Alpha, so there is normally no penalty for using it in the NULL-pointer
case. So, when using rcu_dereference() unconditionally simplifies
the code, it may make sense to "just do it".
Thanx, Paul
^ permalink raw reply
* Re: [RFC][PATCH] identify in_dev_get rcu read-side critical sections
From: Herbert Xu @ 2005-09-30 0:23 UTC (permalink / raw)
To: Suzanne Wood; +Cc: Robert.Olsson, davem, linux-kernel, netdev, paulmck, walpole
In-Reply-To: <200509292359.j8TNxuxD019838@rastaban.cs.pdx.edu>
On Thu, Sep 29, 2005 at 04:59:56PM -0700, Suzanne Wood wrote:
> Sorry to be thinking on-line, but if you mean this:
>
> if (in_dev = rcu_dereference(dev->ip_ptr))
>
> I think that's fine.
Close. What I had in mind is
rcu_read_lock();
in_dev = dev->ip_ptr;
if (in_dev) {
in_dev = rcu_dereference(in_dev);
atomic_inc(&in_dev->refcnt);
}
rcu_read_unlock();
return in_dev;
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: [RFC][PATCH] identify in_dev_get rcu read-side critical sections
From: Herbert Xu @ 2005-09-30 0:21 UTC (permalink / raw)
To: Suzanne Wood; +Cc: Robert.Olsson, davem, linux-kernel, netdev, paulmck, walpole
In-Reply-To: <200509292330.j8TNUSmH019572@rastaban.cs.pdx.edu>
On Thu, Sep 29, 2005 at 04:30:28PM -0700, Suzanne Wood wrote:
>
> > BTW, could you please move the rcu_dereference in in_dev_get()
> > into the if clause? The barrier is not needed when ip_ptr is
> > NULL.
>
> The trouble with that may be that there are three events, the
> dereference, the assignment, and the conditional test. The
> rcu_dereference() is meant to assure deferred destruction
> throughout.
The deferred destruction is guaranteed here by the reference count.
The only purpose served by rcu_dereference() in in_dev_get() is to
prevent the user from seeing pre-initialisation data.
When the pointer is NULL, you can't see any data at all, let alone
pre-initialisation data.
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: [RFC][PATCH] identify in_dev_get rcu read-side critical sections
From: Suzanne Wood @ 2005-09-29 23:59 UTC (permalink / raw)
To: herbert; +Cc: Robert.Olsson, davem, linux-kernel, netdev, paulmck, walpole
Sorry to be thinking on-line, but if you mean this:
if (in_dev = rcu_dereference(dev->ip_ptr))
I think that's fine.
> From suzannew Thu Sep 29 16:39:57 2005
> > From suzannew Thu Sep 29 16:30:28 2005
> > > From: Herbert Xu 30 Sep 2005 07:28
> > > BTW, could you please move the rcu_dereference in in_dev_get()
> > > into the if clause? The barrier is not needed when ip_ptr is
> > > NULL.
> > The trouble with that may be that there are three events, the
> > dereference, the assignment, and the conditional test. The
> > rcu_dereference() is meant to assure deferred destruction
> > throughout.
^ permalink raw reply
* Re: [RFC][PATCH] identify in_dev_get rcu read-side critical sections
From: Suzanne Wood @ 2005-09-29 23:39 UTC (permalink / raw)
To: herbert; +Cc: Robert.Olsson, davem, linux-kernel, netdev, paulmck, walpole
> From suzannew Thu Sep 29 16:30:28 2005
> > From: Herbert Xu 30 Sep 2005 07:28
> > BTW, could you please move the rcu_dereference in in_dev_get()
> > into the if clause? The barrier is not needed when ip_ptr is
> > NULL.
> The trouble with that may be that there are three events, the
> dereference, the assignment, and the conditional test. The
> rcu_dereference() is meant to assure deferred destruction
> throughout.
Sorry, I was thinking in terms of the rcu_read_lock, so this is misstated.
^ permalink raw reply
* Re: [RFC][PATCH] identify in_dev_get rcu read-side critical sections
From: Suzanne Wood @ 2005-09-29 23:30 UTC (permalink / raw)
To: herbert; +Cc: Robert.Olsson, davem, linux-kernel, netdev, paulmck, walpole
> Date: Fri, 30 Sep 2005 07:28:36 +1000
> From: Herbert Xu <herbert@gondor.apana.org.au>
> On Thu, Sep 29, 2005 at 09:02:29AM -0700, Suzanne Wood wrote:
> >
> > The exchange below suggests that it is equally important
> > to have the rcu_dereference() in __in_dev_get(), so the
> > idea of the only difference between in_dev_get and
> > __in_dev_get being the refcnt may be accepted.
> With __in_dev_get() it's the caller's responsibility to ensure
> that RCU works correctly. Therefore if any rcu_dereference is
> needed it should be done by the caller.
This sounds reasonable to me. Does everyone agree?
> Some callers of __in_dev_get() don't need rcu_dereference at all
> because they're protected by the rtnl.
> BTW, could you please move the rcu_dereference in in_dev_get()
> into the if clause? The barrier is not needed when ip_ptr is
> NULL.
The trouble with that may be that there are three events, the
dereference, the assignment, and the conditional test. The
rcu_dereference() is meant to assure deferred destruction
throughout.
Thank you very much for your suggestions.
^ permalink raw reply
* Re: [RFC][PATCH] identify in_dev_get rcu read-side critical sections
From: Herbert Xu @ 2005-09-29 21:28 UTC (permalink / raw)
To: Suzanne Wood; +Cc: paulmck, Robert.Olsson, davem, linux-kernel, netdev, walpole
In-Reply-To: <200509291602.j8TG2TuI015920@rastaban.cs.pdx.edu>
On Thu, Sep 29, 2005 at 09:02:29AM -0700, Suzanne Wood wrote:
>
> The exchange below suggests that it is equally important
> to have the rcu_dereference() in __in_dev_get(), so the
> idea of the only difference between in_dev_get and
> __in_dev_get being the refcnt may be accepted.
With __in_dev_get() it's the caller's responsibility to ensure
that RCU works correctly. Therefore if any rcu_dereference is
needed it should be done by the caller.
Some callers of __in_dev_get() don't need rcu_dereference at all
because they're protected by the rtnl.
BTW, could you please move the rcu_dereference in in_dev_get()
into the if clause? The barrier is not needed when ip_ptr is
NULL.
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
* netfilter hooks for ESP over UDP packets are not invoked
From: Shekhar Kshirsagar @ 2005-09-29 18:25 UTC (permalink / raw)
To: netdev
iptables rules to match ESP packets work fine for raw ESP, but they do
not work for ESP over UDP packets.
Looking into the code, it seems that netfilter hooks are not invoked for
ESP packets that came over UDP.
Does somebody already have a patch to resolve this issue?
Thanks,
Shekhar
^ permalink raw reply
* Re: Possible BUG in IPv4 TCP window handling, all recent 2.4.x/2.6.x kernels
From: David S. Miller @ 2005-09-29 18:16 UTC (permalink / raw)
To: jheffner; +Cc: kuznet, lists, linux-kernel, linux-net, netdev, gautran
In-Reply-To: <200509291204.29393.jheffner@psc.edu>
From: John Heffner <jheffner@psc.edu>
Date: Thu, 29 Sep 2005 12:04:28 -0400
> Has anyone looked at the patch I sent out on Sept 9? It goes a few steps
> further, addressing some additional problems. Original message below.
It's in my inbox pending review, so it's not forgotten :-)
My gut instinct right now is that we should put Alexey's
2-liner in for 2.6.14 et al. then be thinking about your
scheme for 2.6.15
^ permalink raw reply
* Re: [PATCH] net/bluetooth/lib.c: remove unneeded #defines
From: Alexey Dobriyan @ 2005-09-29 18:14 UTC (permalink / raw)
To: Marcel Holtmann; +Cc: Maxim Krasnyansky, bluez-devel, netdev
In-Reply-To: <1128012093.30743.20.camel@localhost.localdomain>
On Thu, Sep 29, 2005 at 06:41:32PM +0200, Marcel Holtmann wrote:
> > -#include <linux/config.h>
> > #include <linux/module.h>
> >
> > #include <linux/kernel.h>
> > -#include <linux/stddef.h>
> > -#include <linux/string.h>
> > #include <asm/errno.h>
> >
> > #include <net/bluetooth/bluetooth.h>
>
> thanks for the patch. Do you have more of them? If yes, then I like to
> cleanup this in one row and not step by step.
Looks like fcntl.h can be dropped too. I'll send a bigger patch.
-------------------------------------------------------
This SF.Net email is sponsored by:
Power Architecture Resource Center: Free content, downloads, discussions,
and more. http://solutions.newsforge.com/ibmarch.tmpl
^ permalink raw reply
* Re: [PATCH] net/bluetooth/lib.c: remove unneeded #defines
From: Marcel Holtmann @ 2005-09-29 16:41 UTC (permalink / raw)
To: Alexey Dobriyan; +Cc: Maxim Krasnyansky, bluez-devel, netdev
In-Reply-To: <20050928202856.GA27645@mipter.zuzino.mipt.ru>
Hi Alexey,
> Signed-off-by: Alexey Dobriyan <adobriyan@gmail.com>
> ---
>
> net/bluetooth/lib.c | 3 ---
> 1 file changed, 3 deletions(-)
>
> --- a/net/bluetooth/lib.c
> +++ b/net/bluetooth/lib.c
> @@ -24,12 +24,9 @@
>
> /* Bluetooth kernel library. */
>
> -#include <linux/config.h>
> #include <linux/module.h>
>
> #include <linux/kernel.h>
> -#include <linux/stddef.h>
> -#include <linux/string.h>
> #include <asm/errno.h>
>
> #include <net/bluetooth/bluetooth.h>
thanks for the patch. Do you have more of them? If yes, then I like to
cleanup this in one row and not step by step.
Regards
Marcel
-------------------------------------------------------
This SF.Net email is sponsored by:
Power Architecture Resource Center: Free content, downloads, discussions,
and more. http://solutions.newsforge.com/ibmarch.tmpl
^ 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