* Re: [RFC 2/2] [IPV6] ADDRCONF: Support RFC3484 configurable address selection policy table.
From: Paul E. McKenney @ 2007-10-31 0:12 UTC (permalink / raw)
To: YOSHIFUJI Hideaki / 吉藤英明; +Cc: netdev
In-Reply-To: <20071030.145254.120595012.yoshfuji@linux-ipv6.org>
On Tue, Oct 30, 2007 at 02:52:54PM +0900, YOSHIFUJI Hideaki / 吉藤英明 wrote:
Looks good from an RCU perspective. A couple questions below about
hlist_for_each_entry_safe().
Thanx, Paul
> Signed-off-by: YOSHIFUJI Hideaki <yoshfuji@linux-ipv6.org>
> ---
> include/linux/if_addrlabel.h | 55 ++++
> include/linux/rtnetlink.h | 7 +
> include/net/addrconf.h | 8 +
> net/ipv6/Makefile | 1 +
> net/ipv6/addrconf.c | 40 +---
> net/ipv6/addrlabel.c | 566 ++++++++++++++++++++++++++++++++++++++++++
> 6 files changed, 646 insertions(+), 31 deletions(-)
>
> diff --git a/include/linux/if_addrlabel.h b/include/linux/if_addrlabel.h
> new file mode 100644
> index 0000000..66978a5
> --- /dev/null
> +++ b/include/linux/if_addrlabel.h
> @@ -0,0 +1,55 @@
> +/*
> + * ifaddrlabel.h - netlink interface for address labels
> + *
> + * Copyright (C)2007 USAGI/WIDE Project, All Rights Reserved.
> + *
> + * Redistribution and use in source and binary forms, with or without
> + * modification, are permitted provided that the following conditions
> + * are met:
> + *
> + * 1. Redistributions of source code must retain the above copyright
> + * notice, this list of conditions and the following disclaimer.
> + * 2. Redistributions in binary form must reproduce the above copyright
> + * notice, this list of conditions and the following disclaimer in the
> + * documentation and/or other materials provided with the distribution.
> + * 3. Neither the name of the project nor the names of its contributors
> + * may be used to endorse or promote products derived from this software
> + * without specific prior written permission.
> + *
> + * THIS SOFTWARE IS PROVIDED BY THE PROJECT AND CONTRIBUTORS ``AS IS'' AND
> + * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
> + * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
> + * ARE DISCLAIMED. IN NO EVENT SHALL THE PROJECT OR CONTRIBUTORS BE LIABLE
> + * FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
> + * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS
> + * OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION)
> + * HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT
> + * LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY
> + * OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
> + * SUCH DAMAGE.
> + *
> + */
> +
> +#ifndef __LINUX_IF_ADDRLABEL_H
> +#define __LINUX_IF_ADDRLABEL_H
> +
> +struct ifaddrlblmsg
> +{
> + __u8 ifal_family; /* Address family */
> + __u8 __ifal_reserved; /* Reserved */
> + __u8 ifal_prefixlen; /* Prefix length */
> + __u8 ifal_flags; /* Flags */
> + __u32 ifal_index; /* Link index */
> + __u32 ifal_seq; /* sequence number */
> +};
> +
> +enum
> +{
> + IFAL_ADDRESS = 1,
> + IFAL_LABEL = 2,
> + __IFAL_MAX
> +};
> +
> +#define IFAL_MAX (__IFAL_MAX - 1)
> +
> +#endif
> diff --git a/include/linux/rtnetlink.h b/include/linux/rtnetlink.h
> index 5bf6182..9a8bf01 100644
> --- a/include/linux/rtnetlink.h
> +++ b/include/linux/rtnetlink.h
> @@ -100,6 +100,13 @@ enum {
> RTM_NEWNDUSEROPT = 68,
> #define RTM_NEWNDUSEROPT RTM_NEWNDUSEROPT
>
> + RTM_NEWADDRLABEL = 72,
> +#define RTM_NEWADDRLABEL RTM_NEWADDRLABEL
> + RTM_DELADDRLABEL,
> +#define RTM_NEWADDRLABEL RTM_NEWADDRLABEL
> + RTM_GETADDRLABEL,
> +#define RTM_GETADDRLABEL RTM_GETADDRLABEL
> +
> __RTM_MAX,
> #define RTM_MAX (((__RTM_MAX + 3) & ~3) - 1)
> };
> diff --git a/include/net/addrconf.h b/include/net/addrconf.h
> index 33b593e..bccc2fe 100644
> --- a/include/net/addrconf.h
> +++ b/include/net/addrconf.h
> @@ -84,6 +84,14 @@ extern void addrconf_leave_solict(struct inet6_dev *idev,
> struct in6_addr *addr);
>
> /*
> + * IPv6 Address Label subsystem (addrlabel.c)
> + */
> +extern int ipv6_addr_label_init(void);
> +extern void ipv6_addr_label_rtnl_register(void);
> +extern u32 ipv6_addr_label(const struct in6_addr *addr,
> + int type, int ifindex);
> +
> +/*
> * multicast prototypes (mcast.c)
> */
> extern int ipv6_sock_mc_join(struct sock *sk, int ifindex,
> diff --git a/net/ipv6/Makefile b/net/ipv6/Makefile
> index 87c23a7..5ffa980 100644
> --- a/net/ipv6/Makefile
> +++ b/net/ipv6/Makefile
> @@ -5,6 +5,7 @@
> obj-$(CONFIG_IPV6) += ipv6.o
>
> ipv6-objs := af_inet6.o anycast.o ip6_output.o ip6_input.o addrconf.o \
> + addrlabel.o \
> route.o ip6_fib.o ipv6_sockglue.o ndisc.o udp.o udplite.o \
> raw.o protocol.o icmp.o mcast.o reassembly.o tcp_ipv6.o \
> exthdrs.o sysctl_net_ipv6.o datagram.o \
> diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
> index 8b408b7..5b6553f 100644
> --- a/net/ipv6/addrconf.c
> +++ b/net/ipv6/addrconf.c
> @@ -876,36 +876,6 @@ static inline int ipv6_saddr_preferred(int type)
> return 0;
> }
>
> -/* static matching label */
> -static inline int ipv6_addr_label(const struct in6_addr *addr, int type,
> - int ifindex)
> -{
> - /*
> - * prefix (longest match) label
> - * -----------------------------
> - * ::1/128 0
> - * ::/0 1
> - * 2002::/16 2
> - * ::/96 3
> - * ::ffff:0:0/96 4
> - * fc00::/7 5
> - * 2001::/32 6
> - */
> - if (type & IPV6_ADDR_LOOPBACK)
> - return 0;
> - else if (type & IPV6_ADDR_COMPATv4)
> - return 3;
> - else if (type & IPV6_ADDR_MAPPED)
> - return 4;
> - else if (addr->s6_addr32[0] == htonl(0x20010000))
> - return 6;
> - else if (addr->s6_addr16[0] == htons(0x2002))
> - return 2;
> - else if ((addr->s6_addr[0] & 0xfe) == 0xfc)
> - return 5;
> - return 1;
> -}
> -
> int ipv6_dev_get_saddr(struct net_device *daddr_dev,
> struct in6_addr *daddr, struct in6_addr *saddr)
> {
> @@ -4182,7 +4152,13 @@ EXPORT_SYMBOL(unregister_inet6addr_notifier);
>
> int __init addrconf_init(void)
> {
> - int err = 0;
> + int err;
> +
> + if ((err = ipv6_addr_label_init()) < 0) {
> + printk(KERN_CRIT "IPv6 Addrconf: cannot initialize default policy table: %d.\n",
> + err);
> + return err;
> + }
>
> /* The addrconf netdev notifier requires that loopback_dev
> * has it's ipv6 private information allocated and setup
> @@ -4233,6 +4209,8 @@ int __init addrconf_init(void)
> __rtnl_register(PF_INET6, RTM_GETMULTICAST, NULL, inet6_dump_ifmcaddr);
> __rtnl_register(PF_INET6, RTM_GETANYCAST, NULL, inet6_dump_ifacaddr);
>
> + ipv6_addr_label_rtnl_register();
> +
> #ifdef CONFIG_SYSCTL
> addrconf_sysctl.sysctl_header =
> register_sysctl_table(addrconf_sysctl.addrconf_root_dir);
> diff --git a/net/ipv6/addrlabel.c b/net/ipv6/addrlabel.c
> new file mode 100644
> index 0000000..719e785
> --- /dev/null
> +++ b/net/ipv6/addrlabel.c
> @@ -0,0 +1,566 @@
> +/*
> + * IPv6 Address Label subsystem
> + * for the IPv6 "Default" Source Address Selection
> + *
> + * Copyright (C)2007 USAGI/WIDE Project
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, write to the Free Software
> + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
> + *
> + */
> +/*
> + * Author:
> + * YOSHIFUJI Hideaki @ USAGI/WIDE Project <yoshfuji@linux-ipv6.org>
> + */
> +
> +#include <linux/kernel.h>
> +#include <linux/list.h>
> +#include <linux/rcupdate.h>
> +#include <linux/in6.h>
> +#include <net/addrconf.h>
> +#include <linux/if_addrlabel.h>
> +#include <linux/netlink.h>
> +#include <linux/rtnetlink.h>
> +
> +#if 0
> +#define ADDRLABEL(x...) printk(x)
> +#else
> +#define ADDRLABEL(x...) do { ; } while(0)
> +#endif
> +
> +/*
> + * Policy Table
> + */
> +struct ip6addrlbl_entry
> +{
> + struct in6_addr prefix;
> + int prefixlen;
> + int ifindex;
> + int addrtype;
> + u32 label;
> + struct hlist_node list;
> + atomic_t refcnt;
> + struct rcu_head rcu;
> +};
> +
> +static struct ip6addrlbl_table
> +{
> + struct hlist_head head;
> + spinlock_t lock;
> + u32 seq;
> +} ip6addrlbl_table;
> +
> +/*
> + * Default policy table (RFC3484 + extensions)
> + *
> + * prefix addr_type label
> + * -------------------------------------------------------------------------
> + * ::1/128 LOOPBACK 0
> + * ::/0 N/A 1
> + * 2002::/16 N/A 2
> + * ::/96 COMPATv4 3
> + * ::ffff:0:0/96 V4MAPPED 4
> + * fc00::/7 N/A 5 ULA (RFC 4193)
> + * 2001::/32 N/A 6 Teredo (RFC 4380)
> + *
> + * Note: 0xffffffff is used if we do not have any policies.
> + */
> +
> +#define IPV6_ADDR_LABEL_DEFAULT 0xffffffffUL
> +
> +static const __initdata struct ip6addrlbl_init_table
> +{
> + const struct in6_addr *prefix;
> + int prefixlen;
> + u32 label;
> +} ip6addrlbl_init_table[] = {
> + { /* ::/0 */
> + .prefix = &in6addr_any,
> + .label = 1,
> + },{ /* fc00::/7 */
> + .prefix = &(struct in6_addr){{{ 0xfc }}},
> + .prefixlen = 7,
> + .label = 5,
> + },{ /* 2002::/16 */
> + .prefix = &(struct in6_addr){{{ 0x20, 0x02 }}},
> + .prefixlen = 16,
> + .label = 2,
> + },{ /* 2001::/32 */
> + .prefix = &(struct in6_addr){{{ 0x20, 0x01 }}},
> + .prefixlen = 32,
> + .label = 6,
> + },{ /* ::ffff:0:0 */
> + .prefix = &(struct in6_addr){{{ [10] = 0xff, [11] = 0xff }}},
> + .prefixlen = 96,
> + .label = 4,
> + },{ /* ::/96 */
> + .prefix = &in6addr_any,
> + .prefixlen = 96,
> + .label = 3,
> + },{ /* ::1/128 */
> + .prefix = &in6addr_loopback,
> + .prefixlen = 128,
> + .label = 0,
> + }
> +};
> +
> +/* Object management */
> +static inline void ip6addrlbl_free(struct ip6addrlbl_entry *p)
> +{
> + kfree(p);
> +}
> +
> +static inline int ip6addrlbl_hold(struct ip6addrlbl_entry *p)
> +{
> + return atomic_inc_not_zero(&p->refcnt);
> +}
> +
> +static inline void ip6addrlbl_put(struct ip6addrlbl_entry *p)
> +{
> + if (atomic_dec_and_test(&p->refcnt))
> + ip6addrlbl_free(p);
> +}
> +
> +static void ip6addrlbl_free_rcu(struct rcu_head *h)
> +{
> + ip6addrlbl_free(container_of(h, struct ip6addrlbl_entry, rcu));
> +}
> +
> +/* Find label */
> +static int __ip6addrlbl_match(struct ip6addrlbl_entry *p,
> + const struct in6_addr *addr,
> + int addrtype, int ifindex)
> +{
> + if (p->ifindex && p->ifindex != ifindex)
> + return 0;
> + if (p->addrtype && p->addrtype != addrtype)
> + return 0;
> + if (!ipv6_prefix_equal(addr, &p->prefix, p->prefixlen))
> + return 0;
> + return 1;
> +}
> +
> +static struct ip6addrlbl_entry *__ipv6_addr_label(const struct in6_addr *addr,
> + int type, int ifindex)
> +{
> + struct hlist_node *pos;
> + struct ip6addrlbl_entry *p;
> + hlist_for_each_entry_rcu(p, pos, &ip6addrlbl_table.head, list) {
> + if (__ip6addrlbl_match(p, addr, type, ifindex))
> + return p;
> + }
> + return NULL;
> +}
> +
> +u32 ipv6_addr_label(const struct in6_addr *addr, int type, int ifindex)
> +{
> + u32 label;
> + struct ip6addrlbl_entry *p;
> +
> + type &= IPV6_ADDR_MAPPED | IPV6_ADDR_COMPATv4 | IPV6_ADDR_LOOPBACK;
> +
> + rcu_read_lock();
> + p = __ipv6_addr_label(addr, type, ifindex);
> + label = p ? p->label : IPV6_ADDR_LABEL_DEFAULT;
> + rcu_read_unlock();
> +
> + ADDRLABEL(KERN_DEBUG "%s(addr=" NIP6_FMT ", type=%d, ifindex=%d) => %08x\n",
> + __FUNCTION__,
> + NIP6(*addr), type, ifindex,
> + label);
> +
> + return label;
> +}
> +
> +/* allocate one entry */
> +struct ip6addrlbl_entry *ip6addrlbl_alloc(const struct in6_addr *prefix,
> + int prefixlen, int ifindex,
> + u32 label)
> +{
> + struct ip6addrlbl_entry *newp;
> + int addrtype;
> +
> + ADDRLABEL(KERN_DEBUG "%s(prefix=" NIP6_FMT ", prefixlen=%d, ifindex=%d, label=%u)\n",
> + __FUNCTION__,
> + NIP6(*prefix), prefixlen,
> + ifindex,
> + (unsigned int)label);
> +
> + addrtype = ipv6_addr_type(prefix) & (IPV6_ADDR_MAPPED | IPV6_ADDR_COMPATv4 | IPV6_ADDR_LOOPBACK);
> +
> + switch (addrtype) {
> + case IPV6_ADDR_MAPPED:
> + if (prefixlen > 96)
> + return ERR_PTR(-EINVAL);
> + if (prefixlen < 96)
> + addrtype = 0;
> + break;
> + case IPV6_ADDR_COMPATv4:
> + if (prefixlen != 96)
> + addrtype = 0;
> + break;
> + case IPV6_ADDR_LOOPBACK:
> + if (prefixlen != 128)
> + addrtype = 0;
> + break;
> + }
> +
> + newp = kmalloc(sizeof(*newp), GFP_KERNEL);
> + if (!newp)
> + return ERR_PTR(-ENOMEM);
> +
> + ipv6_addr_prefix(&newp->prefix, prefix, prefixlen);
> + newp->prefixlen = prefixlen;
> + newp->ifindex = ifindex;
> + newp->addrtype = addrtype;
> + newp->label = label;
> + INIT_HLIST_NODE(&newp->list);
> + atomic_set(&newp->refcnt, 1);
> + return newp;
> +}
> +
> +/* add a label */
> +int __ip6addrlbl_add(struct ip6addrlbl_entry *newp, int replace)
> +{
> + int ret = 0;
> +
> + ADDRLABEL(KERN_DEBUG "%s(newp=%p, replace=%d)\n",
> + __FUNCTION__,
> + newp, replace);
> +
> + if (hlist_empty(&ip6addrlbl_table.head)) {
> + hlist_add_head_rcu(&newp->list, &ip6addrlbl_table.head);
> + } else {
> + struct hlist_node *pos, *n;
> + struct ip6addrlbl_entry *p = NULL;
> + hlist_for_each_entry_safe(p, pos, n,
Why not list_for_each_entry(), given that you exit the look when you
do the replacement below?
> + &ip6addrlbl_table.head, list) {
> + if (p->prefixlen == newp->prefixlen &&
> + p->ifindex == newp->ifindex &&
> + ipv6_addr_equal(&p->prefix, &newp->prefix)) {
> + if (!replace) {
> + ret = -EEXIST;
> + goto out;
> + }
> + hlist_replace_rcu(&p->list, &newp->list);
> + ip6addrlbl_put(p);
> + call_rcu(&p->rcu, ip6addrlbl_free_rcu);
Side note: if you didn't have a "goto out" here, then this would
be vulnerable to the following sequence of events in a CONFIG_PREEMPT_RT
kernel:
1. Preemption occurs here (cannot happen in CONFIG_PREEMPT or
vanilla kernel, but can happen in -rt).
2. Grace period completes.
3. ip6addrlbl_free_rcu() frees up the current element.
4. This element is allocated and used for some entirely different
structure.
5. This task resumes execution, and the hlist_for_each_entry_safe()
uses a pointer to the newly repurposed block of memory. Oops!
> + goto out;
> + } else if ((p->prefixlen == newp->prefixlen && !p->ifindex) ||
> + (p->prefixlen < newp->prefixlen)) {
> + hlist_add_before_rcu(&newp->list, &p->list);
> + goto out;
> + }
> + }
> + hlist_add_after_rcu(&p->list, &newp->list);
> + }
> +out:
> + if (!ret)
> + ip6addrlbl_table.seq++;
> + return ret;
> +}
> +
> +/* add a label */
> +int ip6addrlbl_add(const struct in6_addr *prefix, int prefixlen,
> + int ifindex, u32 label, int replace)
> +{
> + struct ip6addrlbl_entry *newp;
> + int ret = 0;
> +
> + ADDRLABEL(KERN_DEBUG "%s(prefix=" NIP6_FMT ", prefixlen=%d, ifindex=%d, label=%u, replace=%d)\n",
> + __FUNCTION__,
> + NIP6(*prefix), prefixlen,
> + ifindex,
> + (unsigned int)label,
> + replace);
> +
> + newp = ip6addrlbl_alloc(prefix, prefixlen, ifindex, label);
> + if (IS_ERR(newp))
> + return PTR_ERR(newp);
> + spin_lock(&ip6addrlbl_table.lock);
> + ret = __ip6addrlbl_add(newp, replace);
> + spin_unlock(&ip6addrlbl_table.lock);
> + if (ret)
> + ip6addrlbl_free(newp);
> + return ret;
> +}
> +
> +/* remove a label */
> +int __ip6addrlbl_del(const struct in6_addr *prefix, int prefixlen,
> + int ifindex)
> +{
> + struct ip6addrlbl_entry *p = NULL;
> + struct hlist_node *pos, *n;
> + int ret = -ESRCH;
> +
> + ADDRLABEL(KERN_DEBUG "%s(prefix=" NIP6_FMT ", prefixlen=%d, ifindex=%d)\n",
> + __FUNCTION__,
> + NIP6(*prefix), prefixlen,
> + ifindex);
> +
> + hlist_for_each_entry_safe(p, pos, n, &ip6addrlbl_table.head, list) {
Again, why not list_for_each_entry(), given that you exit the look when
you do the replacement below?
> + if (p->prefixlen == prefixlen &&
> + p->ifindex == ifindex &&
> + ipv6_addr_equal(&p->prefix, prefix)) {
> + hlist_del_rcu(&p->list);
> + ip6addrlbl_put(p);
> + call_rcu(&p->rcu, ip6addrlbl_free_rcu);
> + ret = 0;
> + break;
> + }
> + }
> + return ret;
> +}
> +
> +int ip6addrlbl_del(const struct in6_addr *prefix, int prefixlen,
> + int ifindex)
> +{
> + struct in6_addr prefix_buf;
> + int ret;
> +
> + ADDRLABEL(KERN_DEBUG "%s(prefix=" NIP6_FMT ", prefixlen=%d, ifindex=%d)\n",
> + __FUNCTION__,
> + NIP6(*prefix), prefixlen,
> + ifindex);
> +
> + ipv6_addr_prefix(&prefix_buf, prefix, prefixlen);
> + spin_lock(&ip6addrlbl_table.lock);
> + ret = __ip6addrlbl_del(&prefix_buf, prefixlen, ifindex);
> + spin_unlock(&ip6addrlbl_table.lock);
> + return ret;
> +}
> +
> +/* add default label */
> +static __init int ip6addrlbl_init(void)
> +{
> + int err = 0;
> + int i;
> +
> + ADDRLABEL(KERN_DEBUG "%s()\n", __FUNCTION__);
> +
> + for (i = 0; i < ARRAY_SIZE(ip6addrlbl_init_table); i++) {
> + int ret = ip6addrlbl_add(ip6addrlbl_init_table[i].prefix,
> + ip6addrlbl_init_table[i].prefixlen,
> + 0,
> + ip6addrlbl_init_table[i].label, 0);
> + /* XXX: should we free all rules when we catch an error? */
> + if (ret && (!err || err != -ENOMEM))
> + err = ret;
> + }
> + return err;
> +}
> +
> +int __init ipv6_addr_label_init(void)
> +{
> + spin_lock_init(&ip6addrlbl_table.lock);
> +
> + return ip6addrlbl_init();
> +}
> +
> +static const struct nla_policy ifal_policy[IFAL_MAX+1] = {
> + [IFAL_ADDRESS] = { .len = sizeof(struct in6_addr), },
> + [IFAL_LABEL] = { .len = sizeof(u32), },
> +};
> +
> +static int ip6addrlbl_newdel(struct sk_buff *skb, struct nlmsghdr *nlh,
> + void *arg)
> +{
> + struct ifaddrlblmsg *ifal;
> + struct nlattr *tb[IFAL_MAX+1];
> + struct in6_addr *pfx;
> + u32 label;
> + int err = 0;
> +
> + err = nlmsg_parse(nlh, sizeof(*ifal), tb, IFAL_MAX, ifal_policy);
> + if (err < 0)
> + return err;
> +
> + ifal = nlmsg_data(nlh);
> +
> + if (ifal->ifal_family != AF_INET6 ||
> + ifal->ifal_prefixlen > 128)
> + return -EINVAL;
> +
> + if (ifal->ifal_index &&
> + !__dev_get_by_index(&init_net, ifal->ifal_index))
> + return -EINVAL;
> +
> + if (!tb[IFAL_ADDRESS])
> + return -EINVAL;
> +
> + pfx = nla_data(tb[IFAL_ADDRESS]);
> + if (!pfx)
> + return -EINVAL;
> +
> + if (!tb[IFAL_LABEL])
> + return -EINVAL;
> + label = nla_get_u32(tb[IFAL_LABEL]);
> + if (label == IPV6_ADDR_LABEL_DEFAULT)
> + return -EINVAL;
> +
> + switch(nlh->nlmsg_type) {
> + case RTM_NEWADDRLABEL:
> + err = ip6addrlbl_add(pfx, ifal->ifal_prefixlen,
> + ifal->ifal_index, label,
> + nlh->nlmsg_flags & NLM_F_REPLACE);
> + break;
> + case RTM_DELADDRLABEL:
> + err = ip6addrlbl_del(pfx, ifal->ifal_prefixlen,
> + ifal->ifal_index);
> + break;
> + default:
> + err = -EOPNOTSUPP;
> + }
> + return err;
> +}
> +
> +static inline void ip6addrlbl_putmsg(struct nlmsghdr *nlh,
> + int prefixlen, int ifindex, u32 lseq)
> +{
> + struct ifaddrlblmsg *ifal = nlmsg_data(nlh);
> + ifal->ifal_family = AF_INET6;
> + ifal->ifal_prefixlen = prefixlen;
> + ifal->ifal_flags = 0;
> + ifal->ifal_index = ifindex;
> + ifal->ifal_seq = lseq;
> +};
> +
> +static int ip6addrlbl_fill(struct sk_buff *skb,
> + struct ip6addrlbl_entry *p,
> + u32 lseq,
> + u32 pid, u32 seq, int event,
> + unsigned int flags)
> +{
> + struct nlmsghdr *nlh = nlmsg_put(skb, pid, seq, event,
> + sizeof(struct ifaddrlblmsg), flags);
> + if (!nlh)
> + return -EMSGSIZE;
> +
> + ip6addrlbl_putmsg(nlh, p->prefixlen, p->ifindex, lseq);
> +
> + if (nla_put(skb, IFAL_ADDRESS, 16, &p->prefix) < 0 ||
> + nla_put_u32(skb, IFAL_LABEL, p->label) < 0) {
> + nlmsg_cancel(skb, nlh);
> + return -EMSGSIZE;
> + }
> +
> + return nlmsg_end(skb, nlh);
> +}
> +
> +static int ip6addrlbl_dump(struct sk_buff *skb, struct netlink_callback *cb)
> +{
> + struct ip6addrlbl_entry *p;
> + struct hlist_node *pos;
> + int idx = 0, s_idx = cb->args[0];
> + int err;
> +
> + rcu_read_lock();
> + hlist_for_each_entry_rcu(p, pos, &ip6addrlbl_table.head, list) {
> + if (idx >= s_idx) {
> + if ((err = ip6addrlbl_fill(skb, p,
> + ip6addrlbl_table.seq,
> + NETLINK_CB(cb->skb).pid,
> + cb->nlh->nlmsg_seq,
> + RTM_NEWADDRLABEL,
> + NLM_F_MULTI)) <= 0)
> + break;
> + }
> + idx++;
> + }
> + rcu_read_unlock();
> + cb->args[0] = idx;
> + return skb->len;
> +}
> +
> +static inline int ip6addrlbl_msgsize(void)
> +{
> + return (NLMSG_ALIGN(sizeof(struct ifaddrlblmsg))
> + + nla_total_size(16) /* IFAL_ADDRESS */
> + + nla_total_size(4) /* IFAL_LABEL */
> + );
> +}
> +
> +static int ip6addrlbl_get(struct sk_buff *in_skb, struct nlmsghdr* nlh,
> + void *arg)
> +{
> + struct ifaddrlblmsg *ifal;
> + struct nlattr *tb[IFAL_MAX+1];
> + struct in6_addr *addr;
> + u32 lseq;
> + int err = 0;
> + struct ip6addrlbl_entry *p;
> + struct sk_buff *skb;
> +
> + err = nlmsg_parse(nlh, sizeof(*ifal), tb, IFAL_MAX, ifal_policy);
> + if (err < 0)
> + return err;
> +
> + ifal = nlmsg_data(nlh);
> +
> + if (ifal->ifal_family != AF_INET6 ||
> + ifal->ifal_prefixlen != 128)
> + return -EINVAL;
> +
> + if (ifal->ifal_index &&
> + !__dev_get_by_index(&init_net, ifal->ifal_index))
> + return -EINVAL;
> +
> + if (!tb[IFAL_ADDRESS])
> + return -EINVAL;
> +
> + addr = nla_data(tb[IFAL_ADDRESS]);
> + if (!addr)
> + return -EINVAL;
> +
> + rcu_read_lock();
> + p = __ipv6_addr_label(addr, ipv6_addr_type(addr), ifal->ifal_index);
> + if (p && ip6addrlbl_hold(p))
> + p = NULL;
> + lseq = ip6addrlbl_table.seq;
> + rcu_read_unlock();
> +
> + if (!p) {
> + err = -ESRCH;
> + goto out;
> + }
> +
> + if (!(skb = nlmsg_new(ip6addrlbl_msgsize(), GFP_KERNEL))) {
> + ip6addrlbl_put(p);
> + return -ENOBUFS;
> + }
> +
> + err = ip6addrlbl_fill(skb, p, lseq,
> + NETLINK_CB(in_skb).pid, nlh->nlmsg_seq,
> + RTM_NEWADDRLABEL, 0);
> +
> + ip6addrlbl_put(p);
> +
> + if (err < 0) {
> + WARN_ON(err == -EMSGSIZE);
> + kfree_skb(skb);
> + goto out;
> + }
> +
> + err = rtnl_unicast(skb, NETLINK_CB(in_skb).pid);
> +out:
> + return err;
> +}
> +
> +void __init addrlabel_rtnl_register(void)
> +{
> + __rtnl_register(PF_INET6, RTM_NEWADDRLABEL, ip6addrlbl_newdel, NULL);
> + __rtnl_register(PF_INET6, RTM_DELADDRLABEL, ip6addrlbl_newdel, NULL);
> + __rtnl_register(PF_INET6, RTM_GETADDRLABEL, ip6addrlbl_get, ip6addrlbl_dump);
> +}
> +
> --
> 1.4.4.4
>
> -
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: recent sysfs changes cause lots of network device errors
From: Rick Jones @ 2007-10-31 0:02 UTC (permalink / raw)
To: Greg KH; +Cc: Stephen Hemminger, Tejun Heo, David S. Miller, netdev,
Kay Sievers
In-Reply-To: <20071030224043.GA6612@kroah.com>
> Well, this was a bug that no one caught in -mm as we all seem to be
> running with CONFIG_SYSFS_DEPRECATED disabled. There was a long
> discussion on lkml last week, and here's the patch that should fix it
> that is going to Linus in a day or so (it's in my tree, but I'm supposed
> to be on vacation right now...)
>
> Let me know if this works for you or not.
I don't know if it worked for Stephen, but it certainly worked to get rid of the
last three rename-induced stack traces on my system.
rick jones
^ permalink raw reply
* Re: [PATCH] ehea: add kexec support
From: Michael Ellerman @ 2007-10-30 22:50 UTC (permalink / raw)
To: Christoph Raisch
Cc: Jan-Bernd Themann, Jeff Garzik, linux-kernel, linux-ppc,
Marcus Eder, netdev, ossthema, Stefan Roscher, Thomas Q Klein
In-Reply-To: <OF79E7ACE7.5D0488CE-ONC1257384.002E58C0-C1257384.002F8561@de.ibm.com>
[-- Attachment #1: Type: text/plain, Size: 957 bytes --]
On Tue, 2007-10-30 at 09:39 +0100, Christoph Raisch wrote:
>
> Michael Ellerman <michael@ellerman.id.au> wrote on 28.10.2007 23:32:17:
> >
> >
> > How do you plan to support kdump?
> >
>
> When kexec is fully supported kdump should work out of the box
> as for any other ethernet card (if you load the right eth driver).
> There's nothing specific to kdump you have to handle in
> ethernet device drivers.
> Hope I didn't miss anything here...
Perhaps. When we kdump the kernel does not call the reboot notifiers, so
the code Jan-Bernd just added won't get called. So the eHEA resources
won't be freed. When the kdump kernel tries to load the eHEA driver what
will happen?
cheers
--
Michael Ellerman
OzLabs, IBM Australia Development Lab
wwweb: http://michael.ellerman.id.au
phone: +61 2 6212 1183 (tie line 70 21183)
We do not inherit the earth from our ancestors,
we borrow it from our children. - S.M.A.R.T Person
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 189 bytes --]
^ permalink raw reply
* Re: recent sysfs changes cause lots of network device errors
From: Greg KH @ 2007-10-30 22:40 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: Tejun Heo, David S. Miller, netdev, Kay Sievers
In-Reply-To: <20071030150010.79253f01@freepuppy.rosehill>
On Tue, Oct 30, 2007 at 03:00:10PM -0700, Stephen Hemminger wrote:
> It seems that the network device rename done by distro's
> interacts badly with the code in device_rename() in 2.6.24
> (post -rc1). Network devices create sysfs entries where the sd->s_name is
> just a pointer over to the actual buffer in the netdevice.
>
> Prior to calling device_rename, the code in dev_change_name() updates
> the network device name field to the new name. Then when device_rename
> is called it sees that the new name already exists, and dumps out a bunch
> of sysfs warnings.
>
> I already fixed the obvious case of rename to same name, so that isn't the
> problem.
>
> dev_change_name eth0 -> eth4
> [ 46.029555] sysfs: duplicate filename 'eth4' can not be created
> [ 46.029557] WARNING: at fs/sysfs/dir.c:424 sysfs_add_one()
> [ 46.029559]
> [ 46.029560] Call Trace:
> [ 46.029586] [<ffffffff802e60ff>] sysfs_add_one+0xaf/0xf0
> [ 46.029590] [<ffffffff802e7113>] sysfs_create_link+0xa3/0x140
> [ 46.029597] [<ffffffff803b1bb8>] device_rename+0x1d8/0x230
> [ 46.029603] [<ffffffff8040af45>] dev_change_name+0xe5/0x280
> [ 46.029606] [<ffffffff8040b697>] dev_ioctl+0x2b7/0x540
> [ 46.029612] [<ffffffff803fc32d>] sock_ioctl+0x7d/0x250
> [ 46.029618] [<ffffffff802ab33f>] do_ioctl+0x2f/0xa0
> [ 46.029620] [<ffffffff802ab424>] vfs_ioctl+0x74/0x2d0
> [ 46.029624] [<ffffffff8029bde5>] fd_install+0x25/0x60
> [ 46.029626] [<ffffffff802ab711>] sys_ioctl+0x91/0xb0
> [ 46.029631] [<ffffffff8020bbce>] system_call+0x7e/0x83
>
>
> What is the proper usage mode for the rename code?
> Should the underlying structure get changed first or later?
> Or maybe dev_change_name should just not use device_rename
> at all, and just fix the kobject itself?
>
> device_rename() should be smart enough to:
> 1. not get confused if sysfs entry is already changed
> 2. handle the case of rename to same name correctly.
>
> The device control code needs more regression testing before new patches
> are accepted. I understand there is a strong desire to cleanup and eliminate
> the class device stuff, but before going there you need to create
> regression tests for all usages, and not depend on every subsystem
> maintainer to make changes to keep up with your whims.
Well, this was a bug that no one caught in -mm as we all seem to be
running with CONFIG_SYSFS_DEPRECATED disabled. There was a long
discussion on lkml last week, and here's the patch that should fix it
that is going to Linus in a day or so (it's in my tree, but I'm supposed
to be on vacation right now...)
Let me know if this works for you or not.
thanks,
greg k-h
From: Kay Sievers <kay.sievers@vrfy.org>
Subject: Driver Core: fix bug in device_rename() for SYSFS_DEPRECATED=y
From: Kay Sievers <kay.sievers@vrfy.org>
This should fix the sysfs warnings that renaming network devices is
causing to show up with CONFIG_SYSFS_DEPRECATED=y
The code just shouldn't run if class devices are real directories, it's
an update for the symlink in the class directory. Nobody noticed that as
long as the creation of sysfs files silently failed, and we both missed
it before the merge, because we don't run SYSFS_DEPRECATED=y.
Signed-off-by: Kay Sievers <kay.sievers@vrfy.org>
Cc: Larry Finger <Larry.Finger@lwfinger.net>
Cc: David Miller <davem@davemloft.net>
Cc: Rafael J. Wysocki <rjw@sisk.pl>
Cc: Tejun Heo <htejun@gmail.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>
---
drivers/base/core.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
--- a/drivers/base/core.c
+++ b/drivers/base/core.c
@@ -1228,18 +1228,18 @@ int device_rename(struct device *dev, ch
sysfs_remove_link(&dev->parent->kobj, old_class_name);
}
}
-#endif
-
+#else
if (dev->class) {
sysfs_remove_link(&dev->class->subsys.kobj, old_device_name);
error = sysfs_create_link(&dev->class->subsys.kobj, &dev->kobj,
dev->bus_id);
if (error) {
- /* Uh... how to unravel this if restoring can fail? */
dev_err(dev, "%s: sysfs_create_symlink failed (%d)\n",
__FUNCTION__, error);
}
}
+#endif
+
out:
put_device(dev);
^ permalink raw reply
* Re: [PATCH 1/2] Convert /proc/net/ipv6_route to seq_file interface
From: David Miller @ 2007-10-30 22:41 UTC (permalink / raw)
To: adobriyan; +Cc: netdev
In-Reply-To: <20071030131147.GA6210@localhost.sw.ru>
From: Alexey Dobriyan <adobriyan@sw.ru>
Date: Tue, 30 Oct 2007 16:11:47 +0300
> One proc_net_create() user less.
>
> Signed-off-by: Alexey Dobriyan <adobriyan@sw.ru>
Please make the corrections suggested in the feedback
for this patch and resubmit.
Thanks!
^ permalink raw reply
* Re: [PATCH 2/2] Remove /proc/net/ip_vs_lblcr
From: David Miller @ 2007-10-30 22:40 UTC (permalink / raw)
To: adobriyan; +Cc: netdev
In-Reply-To: <20071030131257.GB6210@localhost.sw.ru>
From: Alexey Dobriyan <adobriyan@sw.ru>
Date: Tue, 30 Oct 2007 16:12:57 +0300
> It's under CONFIG_IP_VS_LBLCR_DEBUG option which never existed.
>
> Signed-off-by: Alexey Dobriyan <adobriyan@sw.ru>
Applied, thanks.
^ permalink raw reply
* Re: [patch 1/1][IPV6] resend: remove duplicate call to proc_net_remove
From: David Miller @ 2007-10-30 22:39 UTC (permalink / raw)
To: dlezcano; +Cc: ebiederm, containers, netdev
In-Reply-To: <20071030162450.257974469@mai.toulouse-stg.fr.ibm.com>
From: Daniel Lezcano <dlezcano@fr.ibm.com>
Date: Tue, 30 Oct 2007 17:23:45 +0100
> The file /proc/net/if_inet6 is removed twice.
> First time in:
> inet6_exit
> ->addrconf_cleanup
> And followed a few lines after by:
> inet6_exit
> -> if6_proc_exit
>
> Signed-off-by: Daniel Lezcano <dlezcano@fr.ibm.com>
Applied, thanks Daniel.
^ permalink raw reply
* Re: [patch 1/1][NETNS] resend: fix net released by rcu callback
From: David Miller @ 2007-10-30 22:39 UTC (permalink / raw)
To: dlezcano; +Cc: ebiederm, containers, netdev
In-Reply-To: <20071030162305.458123510@mai.toulouse-stg.fr.ibm.com>
From: Daniel Lezcano <dlezcano@fr.ibm.com>
Date: Tue, 30 Oct 2007 17:21:40 +0100
> When a network namespace reference is held by a network subsystem,
> and when this reference is decremented in a rcu update callback, we
> must ensure that there is no more outstanding rcu update before
> trying to free the network namespace.
>
> In the normal case, the rcu_barrier is called when the network namespace
> is exiting in the cleanup_net function.
>
> But when a network namespace creation fails, and the subsystems are
> undone (like the cleanup), the rcu_barrier is missing.
>
> This patch adds the missing rcu_barrier.
>
> Signed-off-by: Daniel Lezcano <dlezcano@fr.ibm.com>
Applied, thanks Daniel.
^ permalink raw reply
* Re: [patch 1/1][CORE] resend - fix free_netdev on register_netdev failure
From: David Miller @ 2007-10-30 22:38 UTC (permalink / raw)
To: dlezcano; +Cc: netdev
In-Reply-To: <20071030162059.010348834@mai.toulouse-stg.fr.ibm.com>
From: Daniel Lezcano <dlezcano@fr.ibm.com>
Date: Tue, 30 Oct 2007 17:19:26 +0100
> Point 1:
> The unregistering of a network device schedule a netdev_run_todo.
> This function calls dev->destructor when it is set and the
> destructor calls free_netdev.
>
> Point 2:
> In the case of an initialization of a network device the usual code
> is:
> * alloc_netdev
> * register_netdev
> -> if this one fails, call free_netdev and exit with error.
>
> Point 3:
> In the register_netdevice function at the later state, when the device
> is at the registered state, a call to the netdevice_notifiers is made.
> If one of the notification falls into an error, a rollback to the
> registered state is done using unregister_netdevice.
>
> Conclusion:
> When a network device fails to register during initialization because
> one network subsystem returned an error during a notification call
> chain, the network device is freed twice because of fact 1 and fact 2.
> The second free_netdev will be done with an invalid pointer.
>
> Proposed solution:
> The following patch move all the code of unregister_netdevice *except*
> the call to net_set_todo, to a new function "rollback_registered".
>
> The following functions are changed in this way:
> * register_netdevice: calls rollback_registered when a notification fails
> * unregister_netdevice: calls rollback_register + net_set_todo, the call
> order to net_set_todo is changed because it is the
> latest now. Since it justs add an element to a list
> that should not break anything.
>
> Signed-off-by: Daniel Lezcano <dlezcano@fr.ibm.com>
Looks good, applied.
^ permalink raw reply
* Re: [patch 1/1][NETNS] resend: fix net released by rcu callback
From: Eric W. Biederman @ 2007-10-30 22:34 UTC (permalink / raw)
To: Daniel Lezcano; +Cc: davem, containers, netdev
In-Reply-To: <4727A57E.501@fr.ibm.com>
Daniel Lezcano <dlezcano@fr.ibm.com> writes:
> Yes, I had this problem when doing ipv6 isolation for netns49. The ipv6
> subsystem creation failed and the different subsystem where rollbacked in the
> setup_net function.
> When the network namespace was about to be freed in free_net function, I had the
> error with an usage refcount different from zero.
> It appears that was coming from core/neighbour.c
>
> neigh_parms_release
> -> neigh_rcu_free_parms
> -> neigh_parms_put
> -> neigh_parms_destroy
> -> release_net
>
> The free_net function was called before rcu callback neigh_rcu_free_parms.
Ok. Cool. It is good to know that this patch was tested. These kinds
of subtle fixes are always more solid when you can actually test them.
Eric
^ permalink raw reply
* Re: [PATCH] remove claim balance_rr won't reorder on many to one
From: Rick Jones @ 2007-10-30 22:12 UTC (permalink / raw)
To: Jay Vosburgh; +Cc: netdev
In-Reply-To: <5242.1193777750@death>
Jay Vosburgh wrote:
> Rick Jones <rick.jones2@hp.com> wrote:
> [...]
>
>>- Note that this out of order delivery occurs when both the
>>- sending and receiving systems are utilizing a multiple
>>- interface bond. Consider a configuration in which a
>>- balance-rr bond feeds into a single higher capacity network
>>- channel (e.g., multiple 100Mb/sec ethernets feeding a single
>>- gigabit ethernet via an etherchannel capable switch). In this
>>- configuration, traffic sent from the multiple 100Mb devices to
>>- a destination connected to the gigabit device will not see
>>- packets out of order. However, traffic sent from the gigabit
>>- device to the multiple 100Mb devices may or may not see
>>- traffic out of order, depending upon the balance policy of the
>>- switch. Many switches do not support any modes that stripe
>>- traffic (instead choosing a port based upon IP or MAC level
>>- addresses); for those devices, traffic flowing from the
>>- gigabit device to the many 100Mb devices will only utilize one
>>- interface.
>
>
> Rather than simply removing this entirely (because I do think
> there is value in discussion of the reordering aspects of balance-rr),
> I'd rather see something that makes the following points:
>
> 1- the worst reordering is balance-rr to balance-rr, back to
> back. The reordering rate here depends upon (a) the number of slaves
> involved and (b) packet reception scheduling behaviors (packet
> coalescing, NAPI, etc), and thus will vary signficantly, but won't be
> better than case #2.
>
> 2- next worst is "balance-rr many slow" to "single fast", with
> the reordering rate generally being substantially lower than case #1 (it
> looked like your test showed about a 1% reordering rate, if I'm reading
> your data correctly).
>
> 3- For the "single fast" to "balance-rr many" case, going
> through a switch configured for etherchannel "may or may not see traffic
> out of order, depending upon the balance policy of the switch. Many
> switches do not support any modes that stripe traffic (instead choosing
> a port based upon IP or MAC level addresses); for those devices, traffic
> flowing from the [single fast] device to the [balance-rr many] devices
> will only utilize one interface."
I have to wonder if the full description of the different versions of being a
little bit pregnant is worth it. Just saying that using balance-rr will result
in reordering seems much more simple to comprehend. Also, since balance-rr is
strictly an outbound policy, does case three even enter into it - as you say,
that will be up to the switch, which will be doing whatever it was told or felt
like doing regardless of balance-rr on the bond in the host.
>
> [...]
>
>> This mode requires the switch to have the appropriate ports
>>- configured for "etherchannel" or "trunking."
>>+ configured for "etherchannel" or "aggregation." N.B. some
>>+ switches might use the term "trunking" for something other
>>+ than link aggregation.
>
>
> If memory serves, Sun uses the term "trunking" to refer to
> "etherchannel" compatible behavior.
I'm not really all that tied to that part of the change - it is there because I
noticed in one of the HP ITRC forums someone talking about a switch (Cisco?)
where trunking meant something with vlans rather than aggregation.
>
> I'm also hearing "aggregation" used to described 802.3ad
> specifically.
>
> Perhaps text of the form:
>
> This mode requires the switch to have the appropriate ports
> configured for "Etherchannel." Some switches use different terms, so
> the configuration may be called "trunking" or "aggregation." Note that
> both of these terms also have other meanings. For example, "trunking"
> is also used to describe a type of switch port, and "aggregation" or
> "link aggregation" is often used to refer to 802.3ad link aggregation,
> which is compatible with bonding's 802.3ad mode, but not balance-rr.
>
> Thoughts?
Even better would be to be able to start to move away from "etherchannel"
towards the de jure standard's terms, whatever the heck they are :)
rick jones
^ permalink raw reply
* recent sysfs changes cause lots of network device errors
From: Stephen Hemminger @ 2007-10-30 22:00 UTC (permalink / raw)
To: Greg KH, Tejun Heo, David S. Miller; +Cc: netdev
It seems that the network device rename done by distro's
interacts badly with the code in device_rename() in 2.6.24
(post -rc1). Network devices create sysfs entries where the sd->s_name is
just a pointer over to the actual buffer in the netdevice.
Prior to calling device_rename, the code in dev_change_name() updates
the network device name field to the new name. Then when device_rename
is called it sees that the new name already exists, and dumps out a bunch
of sysfs warnings.
I already fixed the obvious case of rename to same name, so that isn't the
problem.
dev_change_name eth0 -> eth4
[ 46.029555] sysfs: duplicate filename 'eth4' can not be created
[ 46.029557] WARNING: at fs/sysfs/dir.c:424 sysfs_add_one()
[ 46.029559]
[ 46.029560] Call Trace:
[ 46.029586] [<ffffffff802e60ff>] sysfs_add_one+0xaf/0xf0
[ 46.029590] [<ffffffff802e7113>] sysfs_create_link+0xa3/0x140
[ 46.029597] [<ffffffff803b1bb8>] device_rename+0x1d8/0x230
[ 46.029603] [<ffffffff8040af45>] dev_change_name+0xe5/0x280
[ 46.029606] [<ffffffff8040b697>] dev_ioctl+0x2b7/0x540
[ 46.029612] [<ffffffff803fc32d>] sock_ioctl+0x7d/0x250
[ 46.029618] [<ffffffff802ab33f>] do_ioctl+0x2f/0xa0
[ 46.029620] [<ffffffff802ab424>] vfs_ioctl+0x74/0x2d0
[ 46.029624] [<ffffffff8029bde5>] fd_install+0x25/0x60
[ 46.029626] [<ffffffff802ab711>] sys_ioctl+0x91/0xb0
[ 46.029631] [<ffffffff8020bbce>] system_call+0x7e/0x83
What is the proper usage mode for the rename code?
Should the underlying structure get changed first or later?
Or maybe dev_change_name should just not use device_rename
at all, and just fix the kobject itself?
device_rename() should be smart enough to:
1. not get confused if sysfs entry is already changed
2. handle the case of rename to same name correctly.
The device control code needs more regression testing before new patches
are accepted. I understand there is a strong desire to cleanup and eliminate
the class device stuff, but before going there you need to create
regression tests for all usages, and not depend on every subsystem
maintainer to make changes to keep up with your whims.
--
Stephen Hemminger <shemminger@linux-foundation.org>
^ permalink raw reply
* Re: Bonding in active-backup mode with arp monitoring on Xen
From: Jay Vosburgh @ 2007-10-30 21:48 UTC (permalink / raw)
To: Tsutomu Fujii; +Cc: netdev
In-Reply-To: <47253487.3010909@nb.jp.nec.com>
Tsutomu Fujii <t-fujii@nb.jp.nec.com> wrote:
[...]
>The network structure is the following.
>
>+-------------------------------+
>| virtual I/F(eth0) | <- have IP address.
>+-------------------------------+
>| virtual bridge(xenbr0) |
>+-------------------------------+
>| bonding(bond0) | <- don't have IP address
>+--------------+-+--------------+
>|phy I/F(peth0)| |phy I/F(peth1)|
>+--------------+ +--------------+
[...]
>How about removing bond_has_ip() from the condition for calling
>bond_arp_send_all() to use bonding with xen?
>When I removed bond_has_ip(), the resulting kernel worked fine and
>active-backup mode works with arp monitoring.
I looked at this a bit. I believe that when you remove the
bond_has_ip() test, the ARP probes sent out will be of the form "ARP
who-has A.B.C.D tell 0.0.0.0", which the linux kernel will interpret as
IPv4 Duplicate Address Detection probes (as a special case in
arp_process()), and generate some type of reply. That reply appears to
be enough to keep the ARP monitor happy.
I'm not sure if this is a solution that will work for any peer
(some peers may not reply to an ARP with an IP source of all zeros). At
first glance, there doesn't seem to be much of a downside, but I'll have
to experiment with it a bit to see if the check should be optional or
simply removed entirely.
-J
---
-Jay Vosburgh, IBM Linux Technology Center, fubar@us.ibm.com
^ permalink raw reply
* Re: [PATCH] net: Saner thash_entries default with much memory
From: David Miller @ 2007-10-30 21:46 UTC (permalink / raw)
To: ak; +Cc: jdelvare, netdev
In-Reply-To: <200710302142.05753.ak@suse.de>
From: Andi Kleen <ak@suse.de>
Date: Tue, 30 Oct 2007 21:42:05 +0100
> I still have my doubts it makes sense to have an own lock for each bucket. It
> would be probably better to just divide the hash value through a factor
> again and then use that to index a smaller lock only table.
Yes, and that's why we do it this way in the routing cache hashes.
^ permalink raw reply
* Re: [patch 1/1][NETNS] resend: fix net released by rcu callback
From: Daniel Lezcano @ 2007-10-30 21:43 UTC (permalink / raw)
To: Eric W. Biederman; +Cc: davem, containers, netdev
In-Reply-To: <m1odegl47t.fsf@ebiederm.dsl.xmission.com>
Eric W. Biederman wrote:
> Daniel Lezcano <dlezcano@fr.ibm.com> writes:
>
>> When a network namespace reference is held by a network subsystem,
>> and when this reference is decremented in a rcu update callback, we
>> must ensure that there is no more outstanding rcu update before
>> trying to free the network namespace.
>>
>> In the normal case, the rcu_barrier is called when the network namespace
>> is exiting in the cleanup_net function.
>>
>> But when a network namespace creation fails, and the subsystems are
>> undone (like the cleanup), the rcu_barrier is missing.
>>
>> This patch adds the missing rcu_barrier.
>
> Looks sane. Did you have any specific failures related to this or was
> this something that was just caught in review?
Yes, I had this problem when doing ipv6 isolation for netns49. The ipv6
subsystem creation failed and the different subsystem where rollbacked
in the setup_net function.
When the network namespace was about to be freed in free_net function, I
had the error with an usage refcount different from zero.
It appears that was coming from core/neighbour.c
neigh_parms_release
-> neigh_rcu_free_parms
-> neigh_parms_put
-> neigh_parms_destroy
-> release_net
The free_net function was called before rcu callback neigh_rcu_free_parms.
^ permalink raw reply
* Re: [PATCH 23/33] netvm: skb processing
From: Peter Zijlstra @ 2007-10-30 21:44 UTC (permalink / raw)
To: Stephen Hemminger
Cc: Linus Torvalds, Andrew Morton, linux-kernel, linux-mm, netdev,
Trond Myklebust
In-Reply-To: <20071030142634.0f00b492@freepuppy.rosehill>
On Tue, 2007-10-30 at 14:26 -0700, Stephen Hemminger wrote:
> On Tue, 30 Oct 2007 17:04:24 +0100
> Peter Zijlstra <a.p.zijlstra@chello.nl> wrote:
>
> > In order to make sure emergency packets receive all memory needed to proceed
> > ensure processing of emergency SKBs happens under PF_MEMALLOC.
> >
> > Use the (new) sk_backlog_rcv() wrapper to ensure this for backlog processing.
> >
> > Skip taps, since those are user-space again.
> >
> > Signed-off-by: Peter Zijlstra <a.p.zijlstra@chello.nl>
> > ---
> > include/net/sock.h | 5 +++++
> > net/core/dev.c | 44 ++++++++++++++++++++++++++++++++++++++------
> > net/core/sock.c | 18 ++++++++++++++++++
> > 3 files changed, 61 insertions(+), 6 deletions(-)
> >
> > Index: linux-2.6/net/core/dev.c
> > ===================================================================
> > --- linux-2.6.orig/net/core/dev.c
> > +++ linux-2.6/net/core/dev.c
> > @@ -1976,10 +1976,23 @@ int netif_receive_skb(struct sk_buff *sk
> > struct net_device *orig_dev;
> > int ret = NET_RX_DROP;
> > __be16 type;
> > + unsigned long pflags = current->flags;
> > +
> > + /* Emergency skb are special, they should
> > + * - be delivered to SOCK_MEMALLOC sockets only
> > + * - stay away from userspace
> > + * - have bounded memory usage
> > + *
> > + * Use PF_MEMALLOC as a poor mans memory pool - the grouping kind.
> > + * This saves us from propagating the allocation context down to all
> > + * allocation sites.
> > + */
> > + if (skb_emergency(skb))
> > + current->flags |= PF_MEMALLOC;
> >
> > /* if we've gotten here through NAPI, check netpoll */
> > if (netpoll_receive_skb(skb))
> > - return NET_RX_DROP;
> > + goto out;
>
> Why the change? doesn't gcc optimize the common exit case anyway?
It needs to unset PF_MEMALLOC at the exit.
> > @@ -2029,19 +2046,31 @@ int netif_receive_skb(struct sk_buff *sk
> >
> > if (ret == TC_ACT_SHOT || (ret == TC_ACT_STOLEN)) {
> > kfree_skb(skb);
> > - goto out;
> > + goto unlock;
> > }
> >
> > skb->tc_verd = 0;
> > ncls:
> > #endif
> >
> > + if (skb_emergency(skb))
> > + switch(skb->protocol) {
> > + case __constant_htons(ETH_P_ARP):
> > + case __constant_htons(ETH_P_IP):
> > + case __constant_htons(ETH_P_IPV6):
> > + case __constant_htons(ETH_P_8021Q):
> > + break;
>
> Indentation is wrong, and hard coding protocol values as spcial case
> seems bad here. What about vlan's, etc?
The other protocols needs analysis on what memory allocations occur
during packet processing, if anything is done that is not yet accounted
for (skb, route cache) then that needs to be added to a reserve, if
there are any paths that could touch user-space, those need to be
handled.
I've started looking at a few others, but its hard and difficult work if
one is not familiar with the protocols.
> > @@ -2063,8 +2093,10 @@ ncls:
> > ret = NET_RX_DROP;
> > }
> >
> > -out:
> > +unlock:
> > rcu_read_unlock();
> > +out:
> > + tsk_restore_flags(current, pflags, PF_MEMALLOC);
> > return ret;
> > }
Its that tsk_restore_flags() there what requires the s/return/goto/
stuff you noted earlier.
> I am still not convinced that this solves the problem well enough
> to be useful. Can you really survive a heavy memory overcommit?
On a machine with mem=128M, I've ran 4 processes of 64M, 2 file backed
with the files on NFS, 2 anonymous. The processes just cycle through the
memory using writes. This is a 100% overcommit.
During these tests I've ran various network loads.
I've shut down the NFS server, waited for say 15 minutes, and restarted
the NFS server, and the machine came back up and continued.
> In other words, can you prove that the added complexity causes the system
> to survive a real test where otherwise it would not?
I've put some statistics in the skb reserve allocations, those are most
definately used. I'm quite certain the machine would lock up solid
without it.
^ permalink raw reply
* Re: [PATCH 23/33] netvm: skb processing
From: Stephen Hemminger @ 2007-10-30 21:26 UTC (permalink / raw)
To: Peter Zijlstra; +Cc: netdev
In-Reply-To: <20071030160914.749995000@chello.nl>
On Tue, 30 Oct 2007 17:04:24 +0100
Peter Zijlstra <a.p.zijlstra@chello.nl> wrote:
> In order to make sure emergency packets receive all memory needed to proceed
> ensure processing of emergency SKBs happens under PF_MEMALLOC.
>
> Use the (new) sk_backlog_rcv() wrapper to ensure this for backlog processing.
>
> Skip taps, since those are user-space again.
>
> Signed-off-by: Peter Zijlstra <a.p.zijlstra@chello.nl>
> ---
> include/net/sock.h | 5 +++++
> net/core/dev.c | 44 ++++++++++++++++++++++++++++++++++++++------
> net/core/sock.c | 18 ++++++++++++++++++
> 3 files changed, 61 insertions(+), 6 deletions(-)
>
> Index: linux-2.6/net/core/dev.c
> ===================================================================
> --- linux-2.6.orig/net/core/dev.c
> +++ linux-2.6/net/core/dev.c
> @@ -1976,10 +1976,23 @@ int netif_receive_skb(struct sk_buff *sk
> struct net_device *orig_dev;
> int ret = NET_RX_DROP;
> __be16 type;
> + unsigned long pflags = current->flags;
> +
> + /* Emergency skb are special, they should
> + * - be delivered to SOCK_MEMALLOC sockets only
> + * - stay away from userspace
> + * - have bounded memory usage
> + *
> + * Use PF_MEMALLOC as a poor mans memory pool - the grouping kind.
> + * This saves us from propagating the allocation context down to all
> + * allocation sites.
> + */
> + if (skb_emergency(skb))
> + current->flags |= PF_MEMALLOC;
>
> /* if we've gotten here through NAPI, check netpoll */
> if (netpoll_receive_skb(skb))
> - return NET_RX_DROP;
> + goto out;
Why the change? doesn't gcc optimize the common exit case anyway?
>
> if (!skb->tstamp.tv64)
> net_timestamp(skb);
> @@ -1990,7 +2003,7 @@ int netif_receive_skb(struct sk_buff *sk
> orig_dev = skb_bond(skb);
>
> if (!orig_dev)
> - return NET_RX_DROP;
> + goto out;
>
> __get_cpu_var(netdev_rx_stat).total++;
>
> @@ -2009,6 +2022,9 @@ int netif_receive_skb(struct sk_buff *sk
> }
> #endif
>
> + if (skb_emergency(skb))
> + goto skip_taps;
> +
> list_for_each_entry_rcu(ptype, &ptype_all, list) {
> if (!ptype->dev || ptype->dev == skb->dev) {
> if (pt_prev)
> @@ -2017,6 +2033,7 @@ int netif_receive_skb(struct sk_buff *sk
> }
> }
>
> +skip_taps:
> #ifdef CONFIG_NET_CLS_ACT
> if (pt_prev) {
> ret = deliver_skb(skb, pt_prev, orig_dev);
> @@ -2029,19 +2046,31 @@ int netif_receive_skb(struct sk_buff *sk
>
> if (ret == TC_ACT_SHOT || (ret == TC_ACT_STOLEN)) {
> kfree_skb(skb);
> - goto out;
> + goto unlock;
> }
>
> skb->tc_verd = 0;
> ncls:
> #endif
>
> + if (skb_emergency(skb))
> + switch(skb->protocol) {
> + case __constant_htons(ETH_P_ARP):
> + case __constant_htons(ETH_P_IP):
> + case __constant_htons(ETH_P_IPV6):
> + case __constant_htons(ETH_P_8021Q):
> + break;
Indentation is wrong, and hard coding protocol values as spcial case
seems bad here. What about vlan's, etc?
> + default:
> + goto drop;
> + }
> +
> skb = handle_bridge(skb, &pt_prev, &ret, orig_dev);
> if (!skb)
> - goto out;
> + goto unlock;
> skb = handle_macvlan(skb, &pt_prev, &ret, orig_dev);
> if (!skb)
> - goto out;
> + goto unlock;
>
> type = skb->protocol;
> list_for_each_entry_rcu(ptype, &ptype_base[ntohs(type)&15], list) {
> @@ -2056,6 +2085,7 @@ ncls:
> if (pt_prev) {
> ret = pt_prev->func(skb, skb->dev, pt_prev, orig_dev);
> } else {
> +drop:
> kfree_skb(skb);
> /* Jamal, now you will not able to escape explaining
> * me how you were going to use this. :-)
> @@ -2063,8 +2093,10 @@ ncls:
> ret = NET_RX_DROP;
> }
>
> -out:
> +unlock:
> rcu_read_unlock();
> +out:
> + tsk_restore_flags(current, pflags, PF_MEMALLOC);
> return ret;
> }
>
> Index: linux-2.6/include/net/sock.h
> ===================================================================
> --- linux-2.6.orig/include/net/sock.h
> +++ linux-2.6/include/net/sock.h
> @@ -523,8 +523,13 @@ static inline void sk_add_backlog(struct
> skb->next = NULL;
> }
>
> +extern int __sk_backlog_rcv(struct sock *sk, struct sk_buff *skb);
> +
> static inline int sk_backlog_rcv(struct sock *sk, struct sk_buff *skb)
> {
> + if (skb_emergency(skb))
> + return __sk_backlog_rcv(sk, skb);
> +
> return sk->sk_backlog_rcv(sk, skb);
> }
>
> Index: linux-2.6/net/core/sock.c
> ===================================================================
> --- linux-2.6.orig/net/core/sock.c
> +++ linux-2.6/net/core/sock.c
> @@ -319,6 +319,24 @@ int sk_clear_memalloc(struct sock *sk)
> }
> EXPORT_SYMBOL_GPL(sk_clear_memalloc);
>
> +#ifdef CONFIG_NETVM
> +int __sk_backlog_rcv(struct sock *sk, struct sk_buff *skb)
> +{
> + int ret;
> + unsigned long pflags = current->flags;
> +
> + /* these should have been dropped before queueing */
> + BUG_ON(!sk_has_memalloc(sk));
> +
> + current->flags |= PF_MEMALLOC;
> + ret = sk->sk_backlog_rcv(sk, skb);
> + tsk_restore_flags(current, pflags, PF_MEMALLOC);
> +
> + return ret;
> +}
> +EXPORT_SYMBOL(__sk_backlog_rcv);
> +#endif
> +
> static int sock_set_timeout(long *timeo_p, char __user *optval, int optlen)
> {
> struct timeval tv;
I am still not convinced that this solves the problem well enough
to be useful. Can you really survive a heavy memory overcommit?
In other words, can you prove that the added complexity causes the system
to survive a real test where otherwise it would not?
--
Stephen Hemminger <shemminger@linux-foundation.org>
^ permalink raw reply
* Re: [PATCH] net: Saner thash_entries default with much memory
From: David Miller @ 2007-10-30 21:11 UTC (permalink / raw)
To: jdelvare; +Cc: ak, netdev
In-Reply-To: <200710301418.27643.jdelvare@suse.de>
From: Jean Delvare <jdelvare@suse.de>
Date: Tue, 30 Oct 2007 14:18:27 +0100
> OK, let's go with (512 * 1024) then. Want me to send an updated patch?
Why submit a patch that's already in Linus's tree :-)
^ permalink raw reply
* Re: [PATCH] remove claim balance_rr won't reorder on many to one
From: Jay Vosburgh @ 2007-10-30 20:55 UTC (permalink / raw)
To: Rick Jones; +Cc: netdev
In-Reply-To: <200710301948.MAA04351@tardy.cup.hp.com>
Rick Jones <rick.jones2@hp.com> wrote:
[...]
>- Note that this out of order delivery occurs when both the
>- sending and receiving systems are utilizing a multiple
>- interface bond. Consider a configuration in which a
>- balance-rr bond feeds into a single higher capacity network
>- channel (e.g., multiple 100Mb/sec ethernets feeding a single
>- gigabit ethernet via an etherchannel capable switch). In this
>- configuration, traffic sent from the multiple 100Mb devices to
>- a destination connected to the gigabit device will not see
>- packets out of order. However, traffic sent from the gigabit
>- device to the multiple 100Mb devices may or may not see
>- traffic out of order, depending upon the balance policy of the
>- switch. Many switches do not support any modes that stripe
>- traffic (instead choosing a port based upon IP or MAC level
>- addresses); for those devices, traffic flowing from the
>- gigabit device to the many 100Mb devices will only utilize one
>- interface.
Rather than simply removing this entirely (because I do think
there is value in discussion of the reordering aspects of balance-rr),
I'd rather see something that makes the following points:
1- the worst reordering is balance-rr to balance-rr, back to
back. The reordering rate here depends upon (a) the number of slaves
involved and (b) packet reception scheduling behaviors (packet
coalescing, NAPI, etc), and thus will vary signficantly, but won't be
better than case #2.
2- next worst is "balance-rr many slow" to "single fast", with
the reordering rate generally being substantially lower than case #1 (it
looked like your test showed about a 1% reordering rate, if I'm reading
your data correctly).
3- For the "single fast" to "balance-rr many" case, going
through a switch configured for etherchannel "may or may not see traffic
out of order, depending upon the balance policy of the switch. Many
switches do not support any modes that stripe traffic (instead choosing
a port based upon IP or MAC level addresses); for those devices, traffic
flowing from the [single fast] device to the [balance-rr many] devices
will only utilize one interface."
[...]
> This mode requires the switch to have the appropriate ports
>- configured for "etherchannel" or "trunking."
>+ configured for "etherchannel" or "aggregation." N.B. some
>+ switches might use the term "trunking" for something other
>+ than link aggregation.
If memory serves, Sun uses the term "trunking" to refer to
"etherchannel" compatible behavior.
I'm also hearing "aggregation" used to described 802.3ad
specifically.
Perhaps text of the form:
This mode requires the switch to have the appropriate ports
configured for "Etherchannel." Some switches use different terms, so
the configuration may be called "trunking" or "aggregation." Note that
both of these terms also have other meanings. For example, "trunking"
is also used to describe a type of switch port, and "aggregation" or
"link aggregation" is often used to refer to 802.3ad link aggregation,
which is compatible with bonding's 802.3ad mode, but not balance-rr.
Thoughts?
-J
---
-Jay Vosburgh, IBM Linux Technology Center, fubar@us.ibm.com
^ permalink raw reply
* Re: [patch 1/1][IPV6] resend: remove duplicate call to proc_net_remove
From: Eric W. Biederman @ 2007-10-30 20:53 UTC (permalink / raw)
To: Daniel Lezcano; +Cc: davem, containers, netdev
In-Reply-To: <20071030162450.257974469@mai.toulouse-stg.fr.ibm.com>
Daniel Lezcano <dlezcano@fr.ibm.com> writes:
> The file /proc/net/if_inet6 is removed twice.
> First time in:
> inet6_exit
> ->addrconf_cleanup
> And followed a few lines after by:
> inet6_exit
> -> if6_proc_exit
>
> Signed-off-by: Daniel Lezcano <dlezcano@fr.ibm.com>
Acked-by: "Eric W. Biederman" <ebiederm@xmission.com>
Looks like a good clean up to me.
> ---
> net/ipv6/addrconf.c | 4 ----
> 1 file changed, 4 deletions(-)
>
> Index: net-2.6/net/ipv6/addrconf.c
> ===================================================================
> --- net-2.6.orig/net/ipv6/addrconf.c
> +++ net-2.6/net/ipv6/addrconf.c
> @@ -4288,8 +4288,4 @@ void __exit addrconf_cleanup(void)
> del_timer(&addr_chk_timer);
>
> rtnl_unlock();
> -
> -#ifdef CONFIG_PROC_FS
> - proc_net_remove(&init_net, "if_inet6");
> -#endif
> }
>
> --
^ permalink raw reply
* Re: [patch 1/1][NETNS] resend: fix net released by rcu callback
From: Eric W. Biederman @ 2007-10-30 20:47 UTC (permalink / raw)
To: Daniel Lezcano; +Cc: davem, containers, netdev
In-Reply-To: <20071030162305.458123510@mai.toulouse-stg.fr.ibm.com>
Daniel Lezcano <dlezcano@fr.ibm.com> writes:
> When a network namespace reference is held by a network subsystem,
> and when this reference is decremented in a rcu update callback, we
> must ensure that there is no more outstanding rcu update before
> trying to free the network namespace.
>
> In the normal case, the rcu_barrier is called when the network namespace
> is exiting in the cleanup_net function.
>
> But when a network namespace creation fails, and the subsystems are
> undone (like the cleanup), the rcu_barrier is missing.
>
> This patch adds the missing rcu_barrier.
Looks sane. Did you have any specific failures related to this or was
this something that was just caught in review?
Eric
^ permalink raw reply
* Re: [2.6 patch] fix drivers/net/wan/lmc/ compilation
From: Roel Kluin @ 2007-10-30 20:45 UTC (permalink / raw)
To: Adrian Bunk; +Cc: Andrew Morton, David S. Miller, netdev, jgarzik, linux-kernel
In-Reply-To: <20071030203136.GQ7227@stusta.de>
Adrian Bunk wrote:
> Documentation/SubmitChecklist, point 1:
>
> <-- snip -->
>
> ...
> CC drivers/net/wan/lmc/lmc_main.o
> /home/bunk/linux/kernel-2.6/git/linux-2.6/drivers/net/wan/lmc/lmc_main.c: In function ‘lmc_ioctl’:
> /home/bunk/linux/kernel-2.6/git/linux-2.6/drivers/net/wan/lmc/lmc_main.c:239: error: expected expression before ‘else’
> ...
> make[5]: *** [drivers/net/wan/lmc/lmc_main.o] Error 1
>
> <-- snip -->
>
> Signed-off-by: Adrian Bunk <bunk@kernel.org>
>
> ---
> d5e92a30491abf073e0a7f4d46b466c7c97f0f61
> diff --git a/drivers/net/wan/lmc/lmc_main.c b/drivers/net/wan/lmc/lmc_main.c
> index 64eb578..37c52e1 100644
> --- a/drivers/net/wan/lmc/lmc_main.c
> +++ b/drivers/net/wan/lmc/lmc_main.c
> @@ -234,7 +234,7 @@ int lmc_ioctl (struct net_device *dev, struct ifreq *ifr, int cmd) /*fold00*/
> sc->lmc_xinfo.Magic1 = 0xDEADBEEF;
>
> if (copy_to_user(ifr->ifr_data, &sc->lmc_xinfo,
> - sizeof(struct lmc_xinfo))) {
> + sizeof(struct lmc_xinfo)))
> ret = -EFAULT;
> else
> ret = 0;
>
I am sorry, my patch broke this and Kristov Provost also noticed this.
See http://lkml.org/lkml/2007/10/30/355
^ permalink raw reply
* Re: [PATCH] net: Saner thash_entries default with much memory
From: Andi Kleen @ 2007-10-30 20:42 UTC (permalink / raw)
To: David Miller; +Cc: jdelvare, netdev
In-Reply-To: <20071030.005758.184980120.davem@davemloft.net>
> Next, machines that service that many sockets typically have them
> mostly with full transmit queues talking to a very slow receiver at
> the other end.
Not sure -- there are likely use cases with lots of idle but connected
sockets.
Also the constraint here is not really how many sockets are served,
but how well the hash function manages to spread them in the table.. I don't
have good data on that.
But still (512 * 1024) sounds reasonable because e.g. in the lots
of idle socket case you're probably fine with the hash chains
having more than one entry worst case because a small working
set will fit in cache and as long as the chains do not end up
very long walking in cache of a short list will be still fast enough.
> So to me (512 * 1024) is a very reasonable limit and (with lockdep
> and spinlock debugging disabled) this makes the EHASH table consume
> 8MB on UP 64-bit and ~12MB on SMP 64-bit systems.
I still have my doubts it makes sense to have an own lock for each bucket. It
would be probably better to just divide the hash value through a factor
again and then use that to index a smaller lock only table.
-Andi
^ permalink raw reply
* [2.6 patch] fix drivers/net/wan/lmc/ compilation
From: Adrian Bunk @ 2007-10-30 20:31 UTC (permalink / raw)
To: Roel Kluin, Andrew Morton, David S. Miller; +Cc: netdev, jgarzik, linux-kernel
Documentation/SubmitChecklist, point 1:
<-- snip -->
...
CC drivers/net/wan/lmc/lmc_main.o
/home/bunk/linux/kernel-2.6/git/linux-2.6/drivers/net/wan/lmc/lmc_main.c: In function ‘lmc_ioctl’:
/home/bunk/linux/kernel-2.6/git/linux-2.6/drivers/net/wan/lmc/lmc_main.c:239: error: expected expression before ‘else’
...
make[5]: *** [drivers/net/wan/lmc/lmc_main.o] Error 1
<-- snip -->
Signed-off-by: Adrian Bunk <bunk@kernel.org>
---
d5e92a30491abf073e0a7f4d46b466c7c97f0f61
diff --git a/drivers/net/wan/lmc/lmc_main.c b/drivers/net/wan/lmc/lmc_main.c
index 64eb578..37c52e1 100644
--- a/drivers/net/wan/lmc/lmc_main.c
+++ b/drivers/net/wan/lmc/lmc_main.c
@@ -234,7 +234,7 @@ int lmc_ioctl (struct net_device *dev, struct ifreq *ifr, int cmd) /*fold00*/
sc->lmc_xinfo.Magic1 = 0xDEADBEEF;
if (copy_to_user(ifr->ifr_data, &sc->lmc_xinfo,
- sizeof(struct lmc_xinfo))) {
+ sizeof(struct lmc_xinfo)))
ret = -EFAULT;
else
ret = 0;
^ permalink raw reply related
* [PATCH] remove claim balance_rr won't reorder on many to one
From: Rick Jones @ 2007-10-30 19:48 UTC (permalink / raw)
To: netdev
Remove the text which suggests that many balance_rr links feeding into
a single uplink will not experience packet reordering.
More up-to-date tests, with 1G links feeding into a switch with a 10G
uplink, using a 2.6.23-rc8 kernel on the system on which the 1G links
were bonded with balance_rr (mode=0) shows that even a many to one
link configuration will experience packet reordering and the attendant
TCP issues involving spurrious retransmissions and the congestion
window. This happens even with a single, simple bulk transfer such as
a netperf TCP_STREAM test. A more complete description of the tests
and results, including tcptrace analysis of packet traces showing the
degree of reordering and such can be found at:
http://marc.info/?l=linux-netdev&m=119101513406349&w=2
Also, note that some switches use the term "trunking" in a context
other than link aggregation.
Signed-off-by: Rick Jones <rick.jones2@hp.com>
---
diff -r 35e54d4beaad Documentation/networking/bonding.txt
--- a/Documentation/networking/bonding.txt Wed Oct 24 05:06:40 2007 +0000
+++ b/Documentation/networking/bonding.txt Mon Oct 29 03:47:19 2007 -0700
@@ -1696,23 +1696,6 @@ balance-rr: This mode is the only mode t
interface's worth of throughput, even after adjusting
tcp_reordering.
- Note that this out of order delivery occurs when both the
- sending and receiving systems are utilizing a multiple
- interface bond. Consider a configuration in which a
- balance-rr bond feeds into a single higher capacity network
- channel (e.g., multiple 100Mb/sec ethernets feeding a single
- gigabit ethernet via an etherchannel capable switch). In this
- configuration, traffic sent from the multiple 100Mb devices to
- a destination connected to the gigabit device will not see
- packets out of order. However, traffic sent from the gigabit
- device to the multiple 100Mb devices may or may not see
- traffic out of order, depending upon the balance policy of the
- switch. Many switches do not support any modes that stripe
- traffic (instead choosing a port based upon IP or MAC level
- addresses); for those devices, traffic flowing from the
- gigabit device to the many 100Mb devices will only utilize one
- interface.
-
If you are utilizing protocols other than TCP/IP, UDP for
example, and your application can tolerate out of order
delivery, then this mode can allow for single stream datagram
@@ -1720,7 +1703,9 @@ balance-rr: This mode is the only mode t
to the bond.
This mode requires the switch to have the appropriate ports
- configured for "etherchannel" or "trunking."
+ configured for "etherchannel" or "aggregation." N.B. some
+ switches might use the term "trunking" for something other
+ than link aggregation.
active-backup: There is not much advantage in this network topology to
the active-backup mode, as the inactive backup devices are all
^ 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