Re: [PATCH net-2.6.25 3/3] Uninline the inet_twsk_put function
Hi David, David Miller schrieb: inet_timewait_sock begins with a struct sock_common which is where the atomic_t is, and: #define tw_refcnt __tw_common.skc_refcnt So you would have to change struct sock_common over to kref, and thus the entire networking, in order to make such a change. Ok, that sounds too much. Many thanks for following up and taking the time to explain it. But you would have seen this instantly if you had spent 5 seconds looking at how these datastructures are defined. Instead you choose to make me do it and explain it to you instead. Sorry, just matched the wrong pattern here :-) Best Regards Ingo Oeser -- To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH net-2.6.25 3/3] Uninline the inet_twsk_put function
Pavel Emelyanov schrieb: This one is not that big, but is widely used: saves 1200 bytes from net/ipv4/built-in.o +void inet_twsk_put(struct inet_timewait_sock *tw) +{ + if (atomic_dec_and_test(tw-tw_refcnt)) { + struct module *owner = tw-tw_prot-owner; + twsk_destructor((struct sock *)tw); +#ifdef SOCK_REFCNT_DEBUG + printk(KERN_DEBUG %s timewait_sock %p released\n, +tw-tw_prot-name, tw); +#endif + kmem_cache_free(tw-tw_prot-twsk_prot-twsk_slab, tw); + module_put(owner); + } +} +EXPORT_SYMBOL_GPL(inet_twsk_put); More correct fix seems to be conversion to kref. Just create out of line inet_twsk_release() containing sth. similiar to the code inside these braces and modify inet_twsk_put() to sth. like this: static inline inet_twsk_put(struct inet_timewait_sock *tw) { kref_put(tw-kref, inet_twsk_release); } David, can you see any reason (e.g. some crazy lock stuff) NOT to do this? Best Regards Ingo Oeser -- To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2]: e1000: avoid lockup durig error recovery
Hi Linas, Linas Vepstas schrieb: Index: linux-2.6.23-rc8-mm1/include/linux/netdevice.h === --- linux-2.6.23-rc8-mm1.orig/include/linux/netdevice.h 2007-09-26 15:07:05.0 -0500 +++ linux-2.6.23-rc8-mm1/include/linux/netdevice.h2007-11-07 17:14:50.0 -0600 @@ -384,6 +384,18 @@ static inline void napi_enable(struct na clear_bit(NAPI_STATE_SCHED, n-state); } +/** + * napi_enabled_p - return non-zero if napi enabled + * @n: napi context + * + * Mnemonic: _p stands for predicate, returning a yes/no + * answer to the question. Call it is_napi_enabled() an nobody will ask :-) + */ +static inline int napi_enabled_p(struct napi_struct *n) And please make it return bool instead of int. Best Regards Ingo Oeser - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 02/05] ipv6: RFC4214 Support
YOSHIFUJI Hideaki / 吉藤英明 schrieb: In article [EMAIL PROTECTED] (at Wed, 7 Nov 2007 10:52:47 -0800), Templin, Fred L [EMAIL PROTECTED] says: + if (((ipv4 = 0x0100) (ipv4 0x0a00)) || + ((ipv4 = 0x0b00) (ipv4 0x7f00)) || + ((ipv4 = 0x8000) (ipv4 0xa9fe)) || + ((ipv4 = 0xa9ff) (ipv4 0xac10)) || + ((ipv4 = 0xac20) (ipv4 0xc0a8)) || + ((ipv4 = 0xc0a9) (ipv4 0xc612)) || + ((ipv4 = 0xc614) (ipv4 0xe000))) eui[0] |= 0x2; Maybe it is I who did not understand. Can you suggest a clean solution? You could write each element as LOOPBACK(), MULTICAST() etc. eui[0] = (!ZERONETO(a) !PRIVATE_10(a) !LINKLOCAL(a) !PRIVATE_172(a) !PRIVATE_192(a) !NETICDEVBENCH(a) !MULTICAST(a)) ? 2 : 0; Oh, yes that's great! Now even *I* can read what this is all about without reading any RFC :-) Please Fred, try to do it that way. Best Regards Ingo Oeser - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 13/24] [IPSEC]: Move x-outer_mode-output out of locked section
Hi Herbert, Herbert Xu schrieb: diff --git a/net/ipv6/xfrm6_mode_ro.c b/net/ipv6/xfrm6_mode_ro.c index a7bc8c6..4a01cb3 100644 --- a/net/ipv6/xfrm6_mode_ro.c +++ b/net/ipv6/xfrm6_mode_ro.c @@ -53,7 +54,9 @@ static int xfrm6_ro_output(struct xfrm_state *x, struct sk_buff *skb) __skb_pull(skb, hdr_len); memmove(ipv6_hdr(skb), iph, hdr_len); + spin_lock_bh(x-lock); x-lastused = get_seconds(); + spin_unlock_bh(x-lock); return 0; } Can you move the retrieval of the seconds outside the spinlock? e.g. tmp = get_seconds(); spin_lock_bh(x-lock); x-lastused = tmp; spin_unlock_bh(x-lock); or is it not really worth it? Best Regards Ingo Oeser - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 02/05] ipv6: RFC4214 Support
Hi Fred, some comments. Templin, Fred L schrieb: From: Fred L. Templin [EMAIL PROTECTED] This is experimental support for the Intra-Site Automatic Tunnel Addressing Protocol (ISATAP) per RFC4214. It uses the SIT module, and is configured using the unmodified ip utility with device names beginning with: isatap. The following diffs are specific to the Linux 2.6.23 kernel distribution. Signed-off-by: Fred L. Templin [EMAIL PROTECTED] --- --- linux-2.6.23/include/net/addrconf.h.orig 2007-10-09 13:31:38.0 -0700 +++ linux-2.6.23/include/net/addrconf.h 2007-10-26 10:49:40.0 -0700 @@ -241,6 +241,34 @@ static inline int ipv6_addr_is_ll_all_ro addr-s6_addr32[3] == htonl(0x0002)); } +#if defined(CONFIG_IPV6_ISATAP) +static inline int ipv6_isatap_eui64(u8 *eui, __be32 *addr) addr is only used for reading, not writing. No need to pass it as a pointer. +{ + __be32 ipv4 = ntohl(*addr); ntohl(be32_value) != be32_value, so the _be32 attribution of ipv4 is wrong here and sparse will scream. + + eui[0] = 0; + + /* Check for RFC3330 global address ranges */ + if (((ipv4 = 0x0100) (ipv4 0x0a00)) || + ((ipv4 = 0x0b00) (ipv4 0x7f00)) || + ((ipv4 = 0x8000) (ipv4 0xa9fe)) || + ((ipv4 = 0xa9ff) (ipv4 0xac10)) || + ((ipv4 = 0xac20) (ipv4 0xc0a8)) || + ((ipv4 = 0xc0a9) (ipv4 0xc612)) || + ((ipv4 = 0xc614) (ipv4 0xe000))) eui[0] |= 0x2; + Instead of converting network to host byte order at runtime and comparing the results to constants, let the compiler convert the constants to network byte order and compare in network order. so use: if (((*addr = htonl(0x0100)) (*addr htonl(0x0a00))) || instead. The compiler will notice that 0x0100 is a constant and will use _constant_htonl() automatically. + eui[1] = 0; eui[2] = 0x5E; eui[3] = 0xFE; + memcpy (eui+4, addr, 4); + return (0); +} Nitpick: return is not a function. Please write return 0; instead. + +static inline int ipv6_addr_is_isatap(const struct in6_addr *addr) +{ + return (addr-s6_addr32[2] == __constant_htonl(0x02005EFE) || + addr-s6_addr32[2] == __constant_htonl(0x5EFE)); +} +#endif The compiler will notice that 0x0100 is a constant and will use _constant_htonl() automatically. Please use simply htonl(). Best Regards Ingo Oeser - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] [POWERPC] Fix region size check in mpc5200 FEC driver
Hi Grant, Grant Likely schrieb: From: Grant Likely [EMAIL PROTECTED] Driver shouldn't complain if the register range is larger than what it expects. This works around failures with some device trees. But maybe the firmware guys like to know about it? May I suggest putting this in front of the other check? if ((mem.end - mem.start + 1) sizeof(struct mpc52xx_fec)) { printk(KERN_DEBUG DRIVER_NAME - gratious resource size (%lx %x), check mpc52xx_devices.c\n, (unsigned long)(mem.end - mem.start + 1), sizeof(struct mpc52xx_fec)); } - if ((mem.end - mem.start + 1) != sizeof(struct mpc52xx_fec)) { + if ((mem.end - mem.start + 1) sizeof(struct mpc52xx_fec)) { printk(KERN_ERR DRIVER_NAME - - invalid resource size (%lx != %x), check mpc52xx_devices.c\n, + - invalid resource size (%lx %x), check mpc52xx_devices.c\n, (unsigned long)(mem.end - mem.start + 1), sizeof(struct mpc52xx_fec)); return -EINVAL; } Best Regards Ingo Oeser - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [linux-usb-devel] [PATCH] USB: net: Fix asix read transfer buffer allocations.
Valentine Barshak schrieb: Oliver Neukum wrote: Am Montag 22 Oktober 2007 schrieb Valentine Barshak: static int asix_mdio_read(struct net_device *netdev, int phy_id, int loc) { struct usbnet *dev = netdev_priv(netdev); + void *buf; u16 res; mutex_lock(dev-phy_mutex); asix_set_sw_mii(dev); + + buf = kmalloc(2, GFP_KERNEL); This is done under lock. Can you allocate the buffer once and reuse it? I think we can use 2 bytes of the usbnet data buffer for this. I'll submit a new patch soon. If this cannot be done for some reason, then you can at least kmalloc() before you do mutex_lock(dev-phy_mutex); and kfree() after you did mutex_unlock(dev-phy_mutex); The reason to can do this, is that buf has a life time limited to this function. The reason you should do this, is that kmalloc(, GFP_KERNEL) is allowed to sleep, which will block the mutex for that time. While this is technically ok, since mutexes can sleep, it is not desireable, since other users of that mutex are blocked until the allocation is done. If you are able to implement the 2 bytes of usbnet data buffer version, please ignore that mail :-) Best Regards Ingo Oeser - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH]: Third (final?) release of Sun Neptune driver
(XMAC_ADDR2, reg2); + } else { + nw64_mac(BMAC_ADDR0, reg0); + nw64_mac(BMAC_ADDR1, reg1); + nw64_mac(BMAC_ADDR2, reg2); + } +} + +static int niu_num_alt_addr(struct niu *np) static int niu_num_alt_addr(const struct niu *np) +{ + if (np-flags NIU_FLAGS_XMAC) + return XMAC_NUM_ALT_ADDR; + else + return BMAC_NUM_ALT_ADDR; +} + [...] +static void niu_set_max_burst(struct niu *np, struct tx_ring_info *rp) +{ + int mtu = np-dev-mtu; + + rp-max_burst = mtu + 32; + if (rp-max_burst 4096) + rp-max_burst = 4096; Why 32 and 4096? (Magic values) +} + ... ok, I stop here, since this drivers is damn big :-) Best Regards Ingo Oeser - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH]: Third (final?) release of Sun Neptune driver
Ingo Oeser schrieb: +static void niu_init_xif(struct niu *); + +static int link_status_10g(struct niu *np, int *link_up_p) +{ + unsigned long flags; + int err, link_up; + + if (np-link_config.loopback_mode != LOOPBACK_DISABLED) + return -EINVAL; + + link_up = 0; + + spin_lock_irqsave(np-lock, flags); + + err = mdio_read(np, np-phy_addr, BCM8704_PMA_PMD_DEV_ADDR, + BCM8704_PMD_RCV_SIGDET); + if (err 0) + return err; missing spin_unlock_irqsave()? I mean spin_unlock_irqrestore() + if (!(err PMD_RCV_SIGDET_GLOBAL)) + goto out; + + err = mdio_read(np, np-phy_addr, BCM8704_PCS_DEV_ADDR, + BCM8704_PCS_10G_R_STATUS); + if (err 0) + return err; missing spin_unlock_irqsave()? I mean spin_unlock_irqrestore() + if (!(err PCS_10G_R_STATUS_BLK_LOCK)) + goto out; + + err = mdio_read(np, np-phy_addr, BCM8704_PHYXS_DEV_ADDR, + BCM8704_PHYXS_XGXS_LANE_STAT); + if (err 0) + return err; + + if (err != (PHYXS_XGXS_LANE_STAT_ALINGED | + PHYXS_XGXS_LANE_STAT_MAGIC | + PHYXS_XGXS_LANE_STAT_LANE3 | + PHYXS_XGXS_LANE_STAT_LANE2 | + PHYXS_XGXS_LANE_STAT_LANE1 | + PHYXS_XGXS_LANE_STAT_LANE0)) + goto out; + + link_up = 1; + np-link_config.active_speed = SPEED_1; + np-link_config.active_duplex = DUPLEX_FULL; + +out: + spin_unlock_irqrestore(np-lock, flags); + + *link_up_p = link_up; + return 0; +} Ok, enough for today... Best Regards Ingo Oeser - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [BUG] tg3 cannot do PXE (loses MAC address) after soft reboot
Michael Chan schrieb: On Fri, 2007-09-14 at 10:14 +0200, Ingo Oeser wrote: Is it enough to parse the first number in the firmware via simple_strtoul()? No, it's not. We need to check the number after the '.' and possibly an alphabet at the end as well. Worse yet, the version may be different for different chips. Ah, I see. I thought the first number is some constantly increasing internal version number from your SCM[1] and the other is the marketing release number. Thank you for explaining this. And that different version for different chips issue make it really hard. I can see that now. We'll see if we can come up with a simple way to detect the problem. That would help your users (like the customers of my employer). Admins and system vendors will love you for that :-) Best Regards Ingo Oeser [1] Source Control Management e.g. cvs, subversion, git - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [BUG] tg3 cannot do PXE (loses MAC address) after soft reboot
Michael Chan schrieb: On Thu, 2007-09-13 at 21:28 +0200, Lucas Nussbaum wrote: Erm, Wouldn't it be possible to print a warning when the driver loads, saying that the firmware is outdated ? It's possible, but would require the driver to parse the version string. The driver currently reports the version string for information and for the human to parse it. Yes, but most humans don't know all valid firmware versions of their components. Is it enough to parse the first number in the firmware via simple_strtoul()? Then we could do this (pseudo-kernel-code :-)): firmare_version = simple_strtoul(version_string, NULL, 0); if (firmware_version oldest_supported) { printk(KERN_WARNING Please upgrade the firmware (you have %s, we need at least %d)\n, version_string, oldest_supported); } else if (firmware_version newest_supported) { printk(KERN_INFO Firmware version %s (latest supported: %d). You might need a newer driver.\n, version_string, oldest_supported); } else if (firmware_version != newest_supported) { dprintk(Firmware version %s (latest supported: %d). You might consider a firmware upgrade.\n, version_string, oldest_supported); } Maybe that mechanism should be global somewhere? Having this kind of information available would help system maintenance in heterogenous hardware environments. Best regards Ingo Oeser - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] ixgbe: Introduce new 10GbE driver for Intel 82598 based PCI Express adapters...
Hi Auke, Kok, Auke schrieb: tg3.c: if ((tp-tg3_flags TG3_FLAG_PCIX_TARGET_HWBUG) || (tp-tg3_flags2 TG3_FLG2_ICH_WORKAROUND)) is obviously _not_ easier to read than if (tp-tg3_flags.pcix_target_hwbug || tp-tg3_flags.ich_workaround) Yes, but what about: static bool tg3_has_pcix_target_hwdebug(const struct tg3 *tp) { return (tp-tg3_flags TG3_FLAG_PCIX_TARGET_HWBUG) != 0; } static bool tg3_has_ich_workaround(const struct tg3 *tp) { return (tp-tg3_flags2 TG3_FLG2_ICH_WORKAROUND) != 0; } if (tg3_has_pcix_target_hwdebug(tp) || tg3_has_ich_workaround(tp)) { /* do foo */ } That is just as nice as bitfields and makes that stuff more readable. This is also a migration path while going from bitfields to flags incrementally. If you find out that two of these tests are always done together you could even test two of them in one. I would say that this method is definately worse for readability. Yes, open coding this bit masking mess IS making code hardly readable! I would much rather then stick with 'bool' instead. If you can afford the space, that is just as good. If you are undecided, try the accessors. You can even write code, which generates them once. Maybe we could get such nice script for generating a set of bitmasks and accessors in the kernel :-) Source would than be a struct definition with bitfields. All points raised be the people here are actually valid and I consider bitfields nice and clear for specification and example code, but avoid them while doing real coding. Best Regards Ingo Oeser - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] [XFRM]: Add module alias for transformation type. (Re: [PATCH 2/2] [IPV6] MIP6: Loadable module support for MIPv6.)
Dear Nakamura-san, [EMAIL PROTECTED] schrieb: This is the third one of MIPv6 module patch. It can be applied after two patches which are already sent to the list. Could you review it? They look good. Thanks for taking the time to clean this up! Acked-by: Ingo Oeser [EMAIL PROTECTED] Best Regards Ingo Oeser - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] [IPV6] MIP6: Loadable module support for MIPv6.
Masahide NAKAMURA schrieb: Ingo Oeser wrote: What about MODULE_ALIAS(xfrm-type-10-60) and MODULE_ALIAS(xfrm-type-10-43) in mip6.c ? Just replace your second patch (Loadable module support) with one, which additionally adds these two lines to mip6.c ... The aliases in modprobe.conf(5) should not be necessary then. If you are really ambitious you can even define a MODULE_ALIAS_XFRM_TYPE macro in include/net/xfrm.h simliar to to MODULE_ALIAS_XFRM_MODE. I prefer to use new macro like XFRM mode to unify XFRM protocols i.e. esp[46].c, ah[46].c, ipcomp[46].c, and mip6.c if we care about it. Can I add it as extensional patch if nobody has a plan to do this yet? ... and provide a third patch to implement this cleanup. That way there are no administrative changes required due to any of your patches and we can defer the global cleanup, if it causes problems or conflicts with other patches in that area. Does this sound like a plan? Regards Ingo Oeser - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] [IPV6] MIP6: Loadable module support for MIPv6.
Hi, [EMAIL PROTECTED] schrieb: From: Masahide NAKAMURA [EMAIL PROTECTED] This patch makes MIPv6 loadable module named mip6. Here is a modprobe.conf(5) example to load it automatically when user application uses XFRM state for MIPv6: alias xfrm-type-10-43 mip6 alias xfrm-type-10-60 mip6 What about MODULE_ALIAS(xfrm-type-10-60) and MODULE_ALIAS(xfrm-type-10-43) in mip6.c ? The aliases in modprobe.conf(5) should not be necessary then. If you are really ambitious you can even define a MODULE_ALIAS_XFRM_TYPE macro in include/net/xfrm.h simliar to to MODULE_ALIAS_XFRM_MODE. Please be sure to discuss CC Herbert Xu then. Best regards Ingo Oeser - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Fix incorrect prototype for ipxrtr_route_packet()
David Miller schrieb: From: David Woodhouse [EMAIL PROTECTED] Date: Fri, 18 May 2007 09:48:32 +0800 On Thu, 2007-05-17 at 15:27 -0700, Andrew Morton wrote: If only we could find some way in which all callers of a function as well as its definition can see the same declaration? Well, building with --combine helps. :) I think it might be useful to be able to optionally to a --combine build on the whole tree with like allyeconfig or something crazy like that. Just as a once-in-a-while thing to catch declaration mismatch and other similar issues. I would LOVE to see a config option or build target for that. At the moment I haven't figured out, how to do this correctly with Kbuild. Regards Ingo Oeser - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [NET] napi: Call __netif_rx_complete in netif_rx_complete
Hi Herbert, Herbert Xu schrieb: [NET] napi: Call __netif_rx_complete in netif_rx_complete This patch kills a little bit of code duplication between the two variants of netif_rx_complete. What about making it out of line? There is nothing the compiler can optimize away here and the code looks bigger than a function call with a single argument. Maybe even make the callers out of line? Best regards Ingo Oeser - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Fwd: [PATCH] [TIPC]: Enhancements to msg_set_bits() routine]
Hi Jon, Jon Paul Maloy schrieb: Ingo Oeser wrote: static inlinevoid msg_set_bits(struct tipc_msg *m, u32 w, u32 pos, __be32 mask, __be32 val) Care to resubmit? I don't mind at all, but I would first like to understand better what this means. If I understand it correctly __be32 literally means big-endian 32-bit integer, but the way it is used doesn't seem to imply this, since both input and output to htonl() often is of that type. Yes, you are right! I mixed up htonl and ntohl :-) Sorry for the confusion! Best Regards Ingo Oeser - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] cfg80211: new wireless config infrastructure
Hi there, John W. Linville schrieb: From: Johannes Berg [EMAIL PROTECTED] --- /dev/null +++ b/net/wireless/core.c @@ -0,0 +1,209 @@ +/* + * This is the linux wireless configuration interface. + * + * Copyright 2006, 2007 Johannes Berg [EMAIL PROTECTED] + */ + +#include linux/if.h +#include linux/module.h +#include linux/err.h +#include linux/mutex.h +#include linux/list.h +#include linux/nl80211.h +#include linux/debugfs.h +#include linux/notifier.h +#include linux/device.h +#include net/genetlink.h +#include net/cfg80211.h +#include net/wireless.h +#include core.h +#include sysfs.h + +/* name for sysfs, %d is appended */ +#define PHY_NAME phy + +MODULE_AUTHOR(Johannes Berg); +MODULE_LICENSE(GPL); +MODULE_DESCRIPTION(wireless configuration support); + +/* RCU might be appropriate here since we usually + * only read the list, and that can happen quite + * often because we need to do it for each command */ +LIST_HEAD(cfg80211_drv_list); +DEFINE_MUTEX(cfg80211_drv_mutex); +static int wiphy_counter; + +/* for debugfs */ +static struct dentry *ieee80211_debugfs_dir; + +/* exported functions */ + +struct wiphy *wiphy_new(struct cfg80211_ops *ops, int sizeof_priv) +{ + struct cfg80211_registered_device *drv; + int alloc_size; + + alloc_size = sizeof(*drv) + sizeof_priv; + + drv = kzalloc(alloc_size, GFP_KERNEL); + if (!drv) + return NULL; + + drv-ops = ops; + + mutex_lock(cfg80211_drv_mutex); + + if (unlikely(wiphy_counter0)) { mutex_unlock(cfg80211_drv_mutex); + /* ugh, wrapped! */ + kfree(drv); + return NULL; + } + drv-idx = wiphy_counter; + + /* give it a proper name */ + snprintf(drv-wiphy.dev.bus_id, BUS_ID_SIZE, + PHY_NAME %d, drv-idx); + + /* now increase counter for the next time */ + wiphy_counter++; + mutex_unlock(cfg80211_drv_mutex); Since drv and its contents are not visible to anyone yet, I suggest the following code flow for that: mutex_lock(cfg80211_drv_mutex); drv-idx = wiphy_counter; /* increase counter for the next time, if id didn't wrap */ if (drv-idx = 0) wiphy_counter++; mutex_unlock(cfg80211_drv_mutex); if (drv-idx 0) { kfree(drv); return NULL; } /* give it a proper name */ snprintf(drv-wiphy.dev.bus_id, BUS_ID_SIZE, PHY_NAME %d, drv-idx); [enqueue to all lists here] Rest looks good so far. Regards Ingo Oeser - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Fwd: [PATCH] [TIPC]: Enhancements to msg_set_bits() routine]
Hi Jon, Jon Paul Maloy schrieb: 2) The code has been optimized to minimize the number of run-time endianness conversion operations by leveraging the fact that the mask (and, in some cases, the value as well) is constant and the necessary conversion can be performed by the compiler. 3) It can be checked by sparse, if you use proper types. diff --git a/net/tipc/msg.h b/net/tipc/msg.h index 62d5490..5c64e55 100644 --- a/net/tipc/msg.h +++ b/net/tipc/msg.h @@ -71,8 +71,11 @@ static inline void msg_set_word(struct tipc_msg *m, u32 w, u32 val) static inline void msg_set_bits(struct tipc_msg *m, u32 w, u32 pos, u32 mask, u32 val) static inlinevoid msg_set_bits(struct tipc_msg *m, u32 w, u32 pos, __be32 mask, __be32 val) Care to resubmit? Best Regards Ingo Oeser - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/5 2.6.21-rc4] l2tp: pppol2tp core
Hi James, James Chapman schrieb: I thought seq_printf handled large output (multiple pages)? I am aware of the page size limitation of raw proc handlers. No. It does detect the limitation, discards your output an lets you try again, when the next invocation of *_show() has enough buffer space. seq_puts() behaves the same. So you simply do this: if (seq_puts(m, text\n)) goto no_space; Regards Ingo Oeser - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2.6] WE-22 : prevent information leak on 64 bit
Hi, Jean Tourrilhes schrieb: diff -u -p linux/include/net/iw_handler.j1.h linux/include/net/iw_handler.h --- linux/include/net/iw_handler.j1.h 2007-03-16 17:36:22.0 -0700 +++ linux/include/net/iw_handler.h2007-03-21 11:01:09.0 -0700 @@ -500,7 +504,11 @@ iwe_stream_add_event(char * stream, /* /* Check if it's possible */ if(likely((stream + event_len) ends)) { iwe-len = event_len; - memcpy(stream, (char *) iwe, event_len); + /* Beware of alignement issues on 64 bits */ + memcpy(stream, (char *) iwe, IW_EV_LCP_PK_LEN); useless cast (void* and char* are already compatible). You just need to cast to char *, if you like to add an byte offset. If not, just don't cast. + memcpy(stream + IW_EV_LCP_LEN, +((char *) iwe) + IW_EV_LCP_LEN, +event_len - IW_EV_LCP_LEN); stream += event_len; Can this be a helper like stream = copy_to_stream(stream, iwe, len); ? Or do the offsets in stream and iwe vary too much for this? } return stream; @@ -521,10 +529,10 @@ iwe_stream_add_point(char * stream, /* /* Check if it's possible */ if(likely((stream + event_len) ends)) { iwe-len = event_len; - memcpy(stream, (char *) iwe, IW_EV_LCP_LEN); + memcpy(stream, (char *) iwe, IW_EV_LCP_PK_LEN); useless cast. memcpy(stream + IW_EV_LCP_LEN, ((char *) iwe) + IW_EV_LCP_LEN + IW_EV_POINT_OFF, -IW_EV_POINT_LEN - IW_EV_LCP_LEN); +IW_EV_POINT_PK_LEN - IW_EV_LCP_PK_LEN); memcpy(stream + IW_EV_POINT_LEN, extra, iwe-u.data.length); stream += event_len; } @@ -574,7 +582,11 @@ iwe_stream_check_add_event(char *stream /* Check if it's possible, set error if not */ if(likely((stream + event_len) ends)) { iwe-len = event_len; - memcpy(stream, (char *) iwe, event_len); + /* Beware of alignement issues on 64 bits */ + memcpy(stream, (char *) iwe, IW_EV_LCP_PK_LEN); useless cast. + memcpy(stream + IW_EV_LCP_LEN, +((char *) iwe) + IW_EV_LCP_LEN, +event_len - IW_EV_LCP_LEN); stream += event_len; } else *perr = -E2BIG; @@ -598,10 +610,10 @@ iwe_stream_check_add_point(char * stream /* Check if it's possible */ if(likely((stream + event_len) ends)) { iwe-len = event_len; - memcpy(stream, (char *) iwe, IW_EV_LCP_LEN); + memcpy(stream, (char *) iwe, IW_EV_LCP_PK_LEN); useless cast. memcpy(stream + IW_EV_LCP_LEN, ((char *) iwe) + IW_EV_LCP_LEN + IW_EV_POINT_OFF, -IW_EV_POINT_LEN - IW_EV_LCP_LEN); +IW_EV_POINT_PK_LEN - IW_EV_LCP_PK_LEN); memcpy(stream + IW_EV_POINT_LEN, extra, iwe-u.data.length); stream += event_len; } else Regards Ingo Oeser - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: L2TP support?
Hi James, James Chapman schrieb: Is there interest in adding L2TP support? Yes, if there is also a user space part somewhere. I have a patch which could be submitted for review. The PPPoL2TP driver presents a PPPoX socket to userspace pppd in the same way as the PPPoE and PPPoATM drivers. The kernel handles all data traffic, while userspace daemons do L2TP and PPP control message processing. Like the pppoe-plugin for pppd? In that thread, I was asked to improve the scalability of my solution by avoiding the socket-per-session usage imposed by the PPPoX model. Since that is imposed by your generic upper layer (PPPoX) this is no valid argument against your code. (Yes, I've read that thread :-)) Shall I post the patch? Yes, please check your patch using the nice checklist in Documentation/SubmitChecklist and do it in suitable chunks according to Documentation/SubmittingPatches I'm looking forward to review it! Best Regards Ingo Oeser - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/6] IrDA: IrLAP raw mode
Hi Samuel, Samuel Ortiz wrote: This patch allows us to bypass the IrDA stack down to the IrLAP level. Sending and receiving frames is done through a character device. This is useful for e.g. doing real IrDA sniffing, testing external IrDA stacks and for Lirc (once I will add the framing disabling switch). Nice! --- /dev/null +++ b/include/net/irda/irlap_raw.h @@ -0,0 +1,27 @@ +/* + * Copyright (C) 2007 Samuel Ortiz ([EMAIL PROTECTED]) + * + * 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, version 2. + * + */ + +#ifndef _IRLAP_RAW_H +#define _IRLAP_RAW_H + +#ifdef CONFIG_IRDA_RAW + +int irlap_raw_recv_frame(struct sk_buff *skb, struct net_device *dev); +int irlap_raw_register_device(struct net_device * dev); +int irlap_raw_unregister_device(struct net_device * dev); + +#else + +#define irlap_raw_recv_frame(skbuff, netdev) +#define irlap_raw_register_device(netdev) +#define irlap_raw_unregister_device(netdev) This stuff is usually done this way (functions, which just check arguments and do nothing): static inline int irlap_raw_recv_frame(struct sk_buff *skb, struct net_device *dev) { if (skb == NULL) return -EINVAL; if (dev-atalk_ptr == NULL) return -ENODEV; return 0; } static inline int irlap_raw_register_device(struct net_device * dev) { if (dev == NULL) return -ENODEV; return 0; } statice inline int irlap_raw_unregister_device(struct net_device * dev) { if (dev == NULL) return -ENODEV; return 0; } --- a/net/irda/irlap_event.c +++ b/net/irda/irlap_event.c @@ -241,6 +241,11 @@ void irlap_do_event(struct irlap_cb *self, IRLAP_EVENT event, if (!self || self-magic != LAP_MAGIC) return; + if (self-raw_mode) + return; +#endif + I would suggest a small helper function here, which compiles into a constant, if raw_mode is not compiled in. like #ifdef CONFIG_IRDA_RAW static inline int irlap_raw_mod(struct irlap_cb *self) { return self-raw_mode; } #else static inline int irlap_raw_mod(struct irlap_cb *self) { return 0; } #endif Best Regards Ingo Oeser - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Funny Routing change since 2.6.16.x
Hi Patrick, Patrick McHardy schrieb: Ingo Oeser wrote: Patrick McHardy schrieb: My guess is that you're using MASQUERADE on ppp0, which since 2.6.14 doesn't exclude locally generated packets anymore, so it translates them to the primary ppp0 address. For replies it works because NAT is already set up for the incoming packet, without masquerading. Your guess is right! Thanks for that hint. Do you have any idea, how to restore the old behavior? I have to, because the ISP cannot assign a different local address and have problems with the new behavior, because that IP adress is an MX entry and the VPN gateway address for several third party vendor tunnels. So changing that is quite an effort. Since these packets already have the proper source address chosen by routing, there is no need to NAT them anymore. So the easiest fix is to exclude them manually from masquerading based on the address. Just did that (iptables -t nat -I POSTROUTING -s $SRCADDR -o ppp0 -j ACCEPT) and it works without any problems. Many thanks for your very fast help! I'm very happy now :-) Do you know any good place, where this can be documented? Best regards Ingo Oeser - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Funny Routing change since 2.6.16.x
Patrick McHardy schrieb: My guess is that you're using MASQUERADE on ppp0, which since 2.6.14 doesn't exclude locally generated packets anymore, so it translates them to the primary ppp0 address. For replies it works because NAT is already set up for the incoming packet, without masquerading. Your guess is right! Thanks for that hint. Do you have any idea, how to restore the old behavior? I have to, because the ISP cannot assign a different local address and have problems with the new behavior, because that IP adress is an MX entry and the VPN gateway address for several third party vendor tunnels. So changing that is quite an effort. Many thanks for your quick answer. Best regards Ingo Oeser - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] HTB O(1) class lookup
Andi Kleen schrieb: On Monday 05 February 2007 11:16, Jarek Poplawski wrote: I wonder, why not try, at least for a while, to do this a compile (menuconfig) option with a comment: recommended for a large number of classes. After hash optimization and some testing, final decisions could be made. There are already far too many obscure CONFIGs. Don't add more. And for people concerned about memory usage: There is always CONFIG_SMALL for such decisions. Maybe one can limit worst case and average memory usage based on this config. The algorithm should stay the same. Regards Ingo Oeser - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/4] skge: WOL support
Hi Stephen, just a nitpick. Stephen Hemminger schrieb: --- skge.orig/drivers/net/skge.c +++ skge/drivers/net/skge.c @@ -132,18 +132,93 @@ static void skge_get_regs(struct net_dev } /* Wake on Lan only supported on Yukon chips with rev 1 or above */ -static int wol_supported(const struct skge_hw *hw) +static u32 wol_supported(const struct skge_hw *hw) { - return !((hw-chip_id == CHIP_ID_GENESIS || - (hw-chip_id == CHIP_ID_YUKON hw-chip_rev == 0))); + if (hw-chip_id == CHIP_ID_YUKON hw-chip_rev != 0) + return WAKE_MAGIC | WAKE_PHY; + else + return 0; +} You can delete that comment, if you write exactly, what the comment says: if (hw-chip_id == CHIP_ID_YUKON hw-chip_rev = 1) return WAKE_MAGIC | WAKE_PHY; else return 0; Regards Ingo Oeser - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] [v3] PA Semi PWRficient Ethernet driver
Hi, Olof Johansson schrieb: [...] +static int pasemi_mac_close(struct net_device *dev) +{ [..] + do { + pci_read_config_dword(mac-dma_pdev, + PAS_DMA_TXCHAN_TCMDSTA(mac-dma_txch), + stat); + } while (stat PAS_DMA_TXCHAN_TCMDSTA_ACT); + + do { + pci_read_config_dword(mac-dma_pdev, + PAS_DMA_RXCHAN_CCMDSTA(mac-dma_rxch), + stat); + } while (stat PAS_DMA_RXCHAN_CCMDSTA_ACT); + + do { + pci_read_config_dword(mac-dma_pdev, + PAS_DMA_RXINT_RCMDSTA(mac-dma_if), + stat); + } while (stat PAS_DMA_RXINT_RCMDSTA_ACT); You might want to write these loops like that: #define MAX_READ_TRIES 1 unsigned int tries; unsigned int state; for(tries=0; tries MAX_READ_TRIES; tries++) { read_stat(stat); if ((stat STATE_FLAG) == 0); break; cond_resched(); } if (stat STATE_FLAG) { dev_err(mac-pdev-dev, Failed to stop device, possible hardware error?\n); /* Panic, disable device, mark unusable, whatever is better than hanging here */ } That way you: - Let you give other processes a chance to run, while you wait for your hardware state to change (if your hardware can tolerate these latencies). - can somehow handle hardware failure and at least give the user a clue what is happening. Regards Ingo Oeser - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH -mm 3/10][RFC] aio: use iov_length instead of ki_left
On Tuesday, 16. January 2007 06:37, Nate Diller wrote: On 1/15/07, Christoph Hellwig [EMAIL PROTECTED] wrote: On Mon, Jan 15, 2007 at 05:54:50PM -0800, Nate Diller wrote: Convert code using iocb-ki_left to use the more generic iov_length() call. No way. We need to reduce the numer of iovec traversals, not adding more of them. ok, I can work on a version of this that uses struct iodesc. Maybe something like this? struct iodesc { struct iovec *iov; unsigned long nr_segs; size_t nbytes; }; I suppose it's worth doing the iodesc thing along with this patchset anyway, since it'll avoid an extra round of interface churn. What about this instead struct iodesc { struct iovec *iov; unsigned long nr_segs; unsigned long seg_limit; size_t nr_bytes; }; That will enable resizeable iodescs with partial completion state and will enable successive filling of an iodesc with iovs. This will be needed anyway. I built an complete short userspace module for that already. I can post and GPLv2 it somewhere, if people are interested. Regards Ingo Oeser - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 45/59] sysctl: C99 convert ctl_tables in drivers/parport/procfs.c
Hi Eric, On Tuesday, 16. January 2007 17:39, Eric W. Biederman wrote: diff --git a/drivers/parport/procfs.c b/drivers/parport/procfs.c index 2e744a2..5337789 100644 --- a/drivers/parport/procfs.c +++ b/drivers/parport/procfs.c @@ -263,50 +263,118 @@ struct parport_sysctl_table { + { + .ctl_name = DEV_PARPORT_BASE_ADDR, + .procname = base-addr, + .data = NULL, + .maxlen = 0, + .mode = 0444, + .proc_handler = do_hardware_base_addr + }, No need to initialize to zero or NULL. Just list any variable, which is NOT zero or NULL. + { + .ctl_name = DEV_PARPORT_AUTOPROBE + 1, + .procname = autoprobe0, + .data = NULL, + .maxlen = 0, + .maxlen = 0444, + .proc_handler = do_autoprobe + }, Typo here? .mode = 0444 makes mor sense. Regards Ingo Oeser - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 31/59] sysctl: C99 convert the ctl_tables in arch/mips/au1000/common/power.c
Hi Eric, On Tuesday, 16. January 2007 17:39, Eric W. Biederman wrote: diff --git a/arch/mips/au1000/common/power.c b/arch/mips/au1000/common/power.c index b531ab7..31256b8 100644 --- a/arch/mips/au1000/common/power.c +++ b/arch/mips/au1000/common/power.c @@ -419,15 +419,41 @@ static int pm_do_freq(ctl_table * ctl, int write, struct file *file, + { + .ctl_name = CTL_UNNUMBERED, + .procname = suspend, + .data = NULL, + .maxlen = 0, + .mode = 0600, + .proc_handler = pm_do_suspend + }, No need for zero initialization for maxlen. + { + .ctl_name = CTL_UNNUMBERED, + .procname = sleep, + .data = NULL, + .maxlen = 0, + .mode = 0600, + .proc_handler = pm_do_sleep + }, dito + { + .ctl_name = CTL_UNNUMBERED, + .procname = freq, + .data = NULL, + .maxlen = 0, + .mode = 0600, + .proc_handler = pm_do_freq + }, + {} }; dito Regards Ingo Oeser - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/3] chelsio: more rx speedup
Stephen Hemminger schrieb: On Tue, 9 Jan 2007 09:42:03 +0100 Ingo Oeser [EMAIL PROTECTED] wrote: Stephen Hemminger schrieb: --- netdev-2.6.orig/drivers/net/chelsio/sge.c +++ netdev-2.6/drivers/net/chelsio/sge.c Please use NET_IP_ALIGN here: Wrong, NET_IP_ALIGN is intended to deal with platforms where alignment of DMA is more important of alignment of structures. Therefore if data is copied, it should always be 2. Ah! Thanks for clearing this up. These magic numbers always make my head spin so I tried to come up with sth. useful :-) If this pattern is present in many drivers, we might have a define or helper for this. Regards Ingo Oeser - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/3] chelsio: more rx speedup
Divy Le Ray schrieb: Stephen Hemminger wrote: On Tue, 9 Jan 2007 09:42:03 +0100 Ingo Oeser [EMAIL PROTECTED] wrote: Stephen Hemminger schrieb: - if (fl-credits drop_thres) { +use_orig_buf: + if (fl-credits 2) { Why 2? What does this magic number mean? No idea, it was there in the original. (as a parameter). The T2 HW behaves nicely when it is guaranteed to have 2 available entries in the rx free list. Can we have a #define for this? That would help people understand this issue more. And don't be shy about errata, all hardware and software out there has them like the humans that made them :-) Regards Ingo Oeser - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH/RFC 01/10] Implement local diversion of IPv4 skbs
Patrick McHardy schrieb: We support bitwise use of the mark everywhere in current kernels, so that shouldn't be a problem anymore. For firewall mark based policy routing to work, one must still disable rp_filter, because this lookup doesn't take the mark into account[1]. So this statement is not quite true, although I believe you are probably right for this case. BTW: This rp_filter=0 requirement isn't even officially documented (e.g. in the LARTC). Regards Ingo Oeser [1] But does take TOS into account for historic (???) reasons. - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
EEPROM infrastructure (was: [PATCH] eeprom_93cx6: Add write support)
Lennart Sorensen schrieb: On Wed, Dec 13, 2006 at 07:56:50PM +0100, Ivo van Doorn wrote: This patch addes support for writing to the eeprom, this also moves some duplicate code into seperate functions. Signed-off-by Ivo van Doorn [EMAIL PROTECTED] Thank you. I will have a try with that to see if I can get that to work with the jsm driver. Too bad the serial drivers don't have any geteeprom/seteeprom standard ioctl's the way ethtool does for network devices. It might be even better to have eeprom writing infrastructure. Many device types come with eeproms today and they implement it per driver or subsystem. On embedded platforms these EEPROMs might even be shared among different devices. So it might be time to generalize this like we did with LEDs. Any comments? Regards Ingo Oeser - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Add support for configuring the PHY connection interface
Hi Andy, Andy Fleming wrote: diff --git a/arch/powerpc/sysdev/fsl_soc.c b/arch/powerpc/sysdev/fsl_soc.c index b4b5b4a..b053370 100644 --- a/arch/powerpc/sysdev/fsl_soc.c +++ b/arch/powerpc/sysdev/fsl_soc.c @@ -211,6 +211,36 @@ static int __init gfar_set_flags(struct return device_flags; } +/* Return the Linux interface mode type based on the + * specification in the device-tree */ +static int __init gfar_get_interface(struct device_node *np) +{ + const char *istr; + int interface = 0; + + istr = get_property(np, interface, NULL); + + if (istr == NULL) + istr = GMII; + + if (!strcasecmp(istr, GMII)) + interface = PHY_INTERFACE_MODE_GMII; + else if (!strcasecmp(istr, MII)) + interface = PHY_INTERFACE_MODE_MII; + else if (!strcasecmp(istr, RGMII)) + interface = PHY_INTERFACE_MODE_RGMII; + else if (!strcasecmp(istr, SGMII)) + interface = PHY_INTERFACE_MODE_SGMII; + else if (!strcasecmp(istr, TBI)) + interface = PHY_INTERFACE_MODE_TBI; + else if (!strcasecmp(istr, RMII)) + interface = PHY_INTERFACE_MODE_RMII; + else if (!strcasecmp(istr, RTBI)) + interface = PHY_INTERFACE_MODE_RTBI; + + return interface; +} If you change the modes into an enum (as suggested by Kumar), you can make a nice static lookup table indexed by mode with a for loop here. Regards Ingo Oeser - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/3] NetXen: Fixed /sys mapping between device and driver
Hi Amit, one minor nitpick: You wrote: diff --git a/drivers/net/netxen/netxen_nic_main.c b/drivers/net/netxen/netxen_nic_main.c index b54ea16..4effb87 100644 --- a/drivers/net/netxen/netxen_nic_main.c +++ b/drivers/net/netxen/netxen_nic_main.c [...] @@ -1040,7 +1041,7 @@ static int netxen_nic_poll(struct net_de netxen_nic_enable_int(adapter); } - return (done ? 0 : 1); + return (!done); return !done; Please lose the braces here (CodingStyle). Just respin or send this change along with later patchsets. Regards Ingo Oeser - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/3] ethtool: flie option to register dump
Hi Stephen, Stephen Hemminger schrieb: Add ability to take old raw dumps from a file and decode them. It is kind of limited because you still need to have same device as the raw file, but useful for maintainers to decode raw dumps. What about putting the (Permanent) MAC into the first 4-6 octets of the hexdump and decode+check+warn using this info first to avoid mixing up files? Maybe only the first 4 octets to enhance privacy and still being able to detect known bad series. This might prevent user errors when sending such files to developers. Regards Ingo Oeser - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] [NET] reduce sizeof(struct flow)
Hi David, David Miller schrieb: I don't like these kinds of patches because %99 of people will never ever realize the savings because distribution vendors will always, unlaterally, enable everything. People producing Linux Appliances DO compile their own kernels. And some distribution vendors are still at 2.6.8-$WHACKY_PATCHES or similiar, which is not usable for many things (e.g. IPsec, SIP behind NAT etc.). But I agree that workings towards smaller size in make allyesconfig is much better :-) Regards Ingo Oeser - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch 2/2] round_jiffies users
Hi Arjan, Arjan van de Ven wrote: Index: linux-2.6.19-rc1-git6/mm/slab.c === --- linux-2.6.19-rc1-git6.orig/mm/slab.c +++ linux-2.6.19-rc1-git6/mm/slab.c @@ -926,7 +926,7 @@ static void __devinit start_cpu_timer(in if (keventd_up() reap_work-func == NULL) { init_reap_node(cpu); INIT_WORK(reap_work, cache_reap, NULL); - schedule_delayed_work_on(cpu, reap_work, HZ + 3 * cpu); + schedule_delayed_work_on(cpu, reap_work, __round_jiffies_relative(HZ, cpu)); } } Did you changed the behavior by intention? You seem to miss the factor 3 here. This hunk should read: --- linux-2.6.19-rc1-git6.orig/mm/slab.c +++ linux-2.6.19-rc1-git6/mm/slab.c @@ -926,7 +926,7 @@ static void __devinit start_cpu_timer(in if (keventd_up() reap_work-func == NULL) { init_reap_node(cpu); INIT_WORK(reap_work, cache_reap, NULL); - schedule_delayed_work_on(cpu, reap_work, HZ + 3 * cpu); + schedule_delayed_work_on(cpu, reap_work, __round_jiffies_relative(HZ, 3 * cpu)); } } In case you apply it: Signed-off-by: Ingo Oese [EMAIL PROTECTED] Regards Ingo Oeser - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Customizable TCP backoff patch
David Miller wrote: At the very least, seconds might not be fine enough granularity for some circumstances. Heck, the default RTO_MIN is 1/5 of a second. :-) I also understand that going to milliseconds or microseconds would make the size of the in-socket struct members an issue again. These things are never easy are they? :-/ Would be, if floating point values would be allowed. Single precision would be enough in this case and its just 32 bits IIRC. I mean floating point values just for the user-kernel ABI and NOT for the internal timer representation. Regards Ingo Oeser - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] natsemi: Messages being noisy
Hi Tim, Hi Jeff, [EMAIL PROTECTED] wrote: On Thu, Sep 14, 2006 at 05:39:08PM +0200, Ingo Oeser wrote: Ingo Oeser wrote: I get the following message when trying to transfer big files (via FTP or SCP) since Linux 2.6.16.27. It didn't happen with Linux 2.6.13.4. [702238.242237] eth1: increased tx threshold, txcfg 0xd0f01008. [702238.242649] eth1: increased tx threshold, txcfg 0xd0f0100a. Is it possible to have the maximum value right from the start? May I tune it somewhere to be the maximum from the start? This will (if I recall) increase transmit latency. It's a fine line. You might tune it up, but this fallback path probably should not be removed.. Will implementing ethtool_ringparam be the right thing for this? Or is there simply no ethtool mechanism for setting up thresholds, yet? If not, I would simply like to have my original pr_debug patch included. The actual value of the tx threshhold can be queried via ethtool -d eth0 already. What do you think? Regards Ingo Oeser - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] natsemi: Messages being noisy
Hi there, Ingo Oeser wrote: I get the following message when trying to transfer big files (via FTP or SCP) since Linux 2.6.16.27. It didn't happen with Linux 2.6.13.4. [702238.242237] eth1: increased tx threshold, txcfg 0xd0f01008. [702238.242649] eth1: increased tx threshold, txcfg 0xd0f0100a. What about putting this message at the message level DEBUG or even under pr_debug()? Now I even know, that this happens together with a small disruption of traffic, which makes applications hang for a second or less. Is it possible to have the maximum value right from the start? May I tune it somewhere to be the maximum from the start? Would you accept patches for this? Regards Ingo Oeser - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] natsemi: Messages being noisy
Hi there, I get the following message when trying to transfer big files (via FTP or SCP) since Linux 2.6.16.27. It didn't happen with Linux 2.6.13.4. [702238.242237] eth1: increased tx threshold, txcfg 0xd0f01008. [702238.242649] eth1: increased tx threshold, txcfg 0xd0f0100a. What about putting this message at the message level DEBUG or even under pr_debug()? This NIC is just used for PPPoE in our setups. The other message tx underrun with maximum tx threshold is much more useful, since it indicates a real problem. So I would suggest the following patch: Signed-off-by: Ingo Oeser [EMAIL PROTECTED] --- linux-2.6.16.28/drivers/net/natsemi.c~ 2006-08-25 22:51:14.0 +0200 +++ linux-2.6.16.28/drivers/net/natsemi.c 2006-09-13 15:26:24.995044665 +0200 @@ -2338,8 +2338,7 @@ if ((np-tx_config TxDrthMask) TX_DRTH_VAL_LIMIT) { np-tx_config += TX_DRTH_VAL_INC; if (netif_msg_tx_err(np)) - printk(KERN_NOTICE - %s: increased tx threshold, txcfg %#08x.\n, + pr_debug(%s: increased tx threshold, txcfg %#08x.\n, dev-name, np-tx_config); } else { if (netif_msg_tx_err(np)) - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Regarding offloading IPv6 addrconf and ndisc
Hi, David Miller wrote: Developer momentum means that the kernel is likely to get fixed whereas the userland component will more likely rot and not get fixed. So in this sense resiliency does depend upon something being in the kernel or not. I can only agree here. Lots of users use their own kernels instead of distribution kernels. Much less users divert core software from their distribution. The big binary called bzImage/vmlinux whatever is a huge usability advantage here :-) Regards Ingo Oeser - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH][ebtables][vlan] ebt_vlan_t target
Hi Dawid, Dawid Ciężarkiewicz schrieb: my name is Dawid Ciezarkiewicz. As a part of my daily job I was to write kernel module for ebtables to let linux bridges change vlan ids in fly using logic provided by ebtables matches. After hours of tries and kernel learning, reading and googlin' I've finally come to the place where I've got working module that does what I want. I'm looking for comments of more advanced linux developers. Please note that this is my first linux patch I've ever made. First of all: You should not implemented --vlan-target. Always return EBT_CONTINUE. That saves a lot of (duplicated) code (you can express the same using some more rules) while keeping the same flexibility level. Rules for transforming/mangling and decision rules should be seperate. diff -Nur linux-2.6.17.orig/net/bridge/netfilter/Kconfig linux-2.6.17/net/bridge/netfilter/Kconfig --- linux-2.6.17.orig/net/bridge/netfilter/Kconfig2006-06-18 03:49:35.0 +0200 +++ linux-2.6.17/net/bridge/netfilter/Kconfig 2006-06-28 20:48:27.0 +0200 @@ -165,6 +165,15 @@ To compile it as a module, choose M here. If unsure, say N. +config BRIDGE_EBT_VLAN_T + tristate ebt: vlan target support + depends on BRIDGE_NF_EBTABLES + help + This option adds the vlan target. + + To compile it as a module, choose M here. If unsure, say N. + + Please put your nice explanations to change vlan ids in fly from your email here. And short ebtables example for the common use case might help, too. But only, if it is not more than two lines or such. Otherwise omit it. I cannot comment on rest of the patch and hope other people will do :-) Regards Ingo Oeser - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [IOC3] IP27: Really set PCI64_ATTR_VIRTUAL, not PCI64_ATTR_PREC.
Hi Ralf, Ralf Baechle : IOC3's homegrown DMA mapping functions that are used to optimize things a little on IP27 set the wrong bit. What about using a symbol instead of magic numbers? That way one at least sees the intention of the coder. Signed-off-by: Ralf Baechle [EMAIL PROTECTED] diff --git a/drivers/net/ioc3-eth.c b/drivers/net/ioc3-eth.c index ae71ed5..e76e6e7 100644 --- a/drivers/net/ioc3-eth.c +++ b/drivers/net/ioc3-eth.c @@ -145,7 +145,7 @@ static inline struct sk_buff * ioc3_allo static inline unsigned long ioc3_map(void *ptr, unsigned long vdev) { #ifdef CONFIG_SGI_IP27 - vdev = 58; /* Shift to PCI64_ATTR_VIRTUAL */ + vdev = 57; /* Shift to PCI64_ATTR_VIRTUAL */ So please use a symbolic value here. Regards Ingo Oeser - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
[BUG 6480]Re: Asus K8N-VM Motherboard Ethernet Problem
Hi James, On Monday, 29. May 2006 19:48, James Courtier-Dutton wrote: I can concur that the forcedeth is unreliable on nvidia based motherboards. I have a ethernet device that works with forcedeth. :00:0a.0 Bridge: nVidia Corporation CK804 Ethernet Controller (rev a3) :00:0a.0 0680: 10de:0057 (rev a3) Subsystem: 147b:1c12 Flags: 66MHz, fast devsel, IRQ 11 Memory at fe02f000 (32-bit, non-prefetchable) [size=4K] I/O ports at fc00 [size=8] Capabilities: [44] Power Management version 2 It works in that it can actually send and receive packets. The problems are: 1) one cannot rmmod the forcedeth module. Even after ifdown etc. 2) the machine hangs randomly. Before someone asks, the MB has no serial port, so no stack trace available. 3) The netconsole fails to function with it. I have installed a standard PCI based intel ethernet card, and only use that. Without the forcedeth loaded, no hangs since. So, although I can confirm that there are certainly problems with the forcedeth driver, without a serial port, I am at a loss at how I might help diagnose the problem and fix it. This sounds like http://bugzilla.kernel.org/show_bug.cgi?id=6480 Maybe we can help to resolve this. I already stored a lot of info in the bugzilla entry. Could you please describe your setup further? I'm interested on the devices behind your nvidia NIC, since we only have it at one customers mail server behind a SonicWall, so we cannot really try a lot. For all other customers it works. I'm a bit lost on the cause. I've seen 4% collisions, which might reduce performance, but should not stop the transmitter forever. PS: CC'ed some more relevant addresses to increase awareness. Regards Ingo Oeser - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/4] myri10ge - Driver core
Hi there, Benjamin Herrenschmidt wrote: On Wed, 2006-05-24 at 01:39 +1000, Anton Blanchard wrote: +#ifdef CONFIG_MTRR + mgp-mtrr = mtrr_add(mgp-iomem_base, mgp-board_span, + MTRR_TYPE_WRCOMB, 1); +#endif ... + mgp-sram = ioremap(mgp-iomem_base, mgp-board_span); Not sure how we are meant to specify write through in drivers. Any ideas Ben? No proper interface exposed, he'll have to do an #ifdef powerpc here or such and use __ioremap with explicit page attributes. I have a hack to do that automatically for memory covered by prefetchable PCI BARs when mmap'ing from userland but not for kernel ioremap. Stupid question: pci_iomap() is NOT what you are looking for, right? Implementation is at the end of lib/iomap.c Regards Ingo Oeser - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Safe remote kernel install howto (Re: [Bugme-new] [Bug 6613] New: iptables broken on 32-bit PReP (ARCH=ppc))
Hi Meelis, Unfortunatlety, 2.6.15 does not boot on this machine so I'm locked out remotely at the moment. Here it my paranoid boot setup: 1. Use lilo -R new-kernel, to boot a kernel only once and reboot the default kernel next time. 2. Force reboot on any panic after 10 seconds: append=panic=10 in /etc/lilo.conf 3. Schedule automatic reboot in case of impossible login echo /bin/sync; /sbin/reboot -f |at now + 15min 4. Put sysctl -w kernel.panic_on_oops=1 as early as possible in your boot scripts[1]. And now reboot into the new kernel, try to login and delete the reboot cronjob. If this doesn't work, just wait 15min and have the last stable kernel booted automatically. This method saved me and our customers a lot of time already :-) Regards Ingo Oeser [1] This should be the default and should be disabled by the init scripts as soon as we reach the desired runlevel (S99oops_not_fatal). - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Safe remote kernel install howto (Re: [Bugme-new] [Bug 6613] New: iptables broken on 32-bit PReP (ARCH=ppc))
Hi Andi, Andi Kleen wrote: 4. Put sysctl -w kernel.panic_on_oops=1 as early as possible in your boot scripts[1]. You can as well boot with oops=panic Only on x86_64 as of Linux 2.6.16. But maybe this could be put into kernel/panic.c instead :-) Regards Ingo Oeser - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: KERNEL: assertion (!sk-sk_forward_alloc) failed at net/* (again)
Hi Chris, here are some steps to narrow it down. 1. try the latest kernel first (2.6.16.16). This BUG should be fixed there. 2. try without grsecurity patch 3. if it still persists: Please provide more information about your setup before submitting a bug. lspci -vvv and your .config would be the minimum some ethtool outputs (ethtool -k ethX) would help here. 4. If you like to have it resolved very fast, please try git-bisect. This can take a lot of time (several recompiles and reboots needed!). Happy Bug hunting! Regards Ingo Oeser - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] expose simplified skb_checksum_recalc
Hi Stephen, Stephen Hemminger wrote: Many users of skb_checksum_help() are just using it to recalculate outbound checksum, so why not expose the interface in a more useful way. Suggested by Ingo Oeser. You are damn fast Stephen :-) That's even better and improves a lot on documentation and code placement for free. Thanks Regards Ingo Oeser - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC 1/3] [IPSEC] xfrm: Undo afinfo lock proliferation
Hi Herbert, Herbert Xu schrieb: It's just moving an existing line down if you look further up in the patch. Of couse I would have nothing against a patch that replaced 256 with IPPROTO_MAX or something. Ahh, that's the actual meaning here... However, I prefer to not mix clean-ups with substantive changes so if you have the time please post a separate patch for this. Otherwise I'll do a patch when I resubmit this. Would be nice to include such a patch in this patchset, since you seem to be able to decipher that magic value :-) Thanks Regards Ingo Oeser - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Disabling TCP Treason uncloaked
Herbert Xu wrote: BTW, this message is already under net_ratelimit so I don't see any urgency in getting rid of it completely. If we're going down the path of disabling it, we probably should go for something more global rather than a sysctl that controls this one message. Already there: /proc/sys/net/core/{message_cost,message_burst} Just set burst to 0 and cost to a very big value to basically supppress all net_ratelimit()ed messages. Or did you think of sth. else? Regards Ingo Oeser - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
[Bug 6480] Transmitter of CK804 hangs hard, needs power cycle to recover
Hi Manfred, I filed BUG 6480 describing the problem and providing lots of info. If you need more info, just ask. At the moment I have no idea about the reasons. Except Crossover-Cabling vs. using a switch. The machine is a production machine, so I cannot test kernels and patches. But I have several machines with identical hardware where I can test this. Once we can reproduce it without a Sonicwall, I can build a test setup using any kernel hackery required to resolve the issue :-) Could you please contact the right nVIDIA people, if needed? PS: Stephen, you are not CC'ed from bugzilla and agreed to help out with net driver maintenance so I CC'ed you here manually. Regards Ingo Oeser - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 7/32] rt2x00: make vals static
Sie schrieben: From: Ivo van Doorn [EMAIL PROTECTED] The vals[] arrays in *_init_hw_channels can be made static to optimize memory and reduce stack size. What about static const? They are also constants, right? But please try first, if this helps in terms of code size. Regards Ingo Oeser - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 17/32] rt2x00: Put net_device structure in data_ring
Hi Ivo, Ivo van Doorn wrote: diff -uprN wireless-dev-rt2x00/drivers/net/wireless/d80211/rt2x00/rt2400pci.c wireless-dev-rt2x00-patch/drivers/net/wireless/d80211/rt2x00/rt2400pci.c --- wireless-dev-rt2x00/drivers/net/wireless/d80211/rt2x00/rt2400pci.c 2006-04-27 21:48:21.0 +0200 +++ wireless-dev-rt2x00-patch/drivers/net/wireless/d80211/rt2x00/rt2400pci.c 2006-04-27 21:49:08.0 +0200 @@ -760,10 +760,8 @@ rt2400pci_write_tx_desc( static void rt2400pci_beacondone(void *data) { - struct data_ring*ring = (struct data_ring*)data; - struct rt2x00_pci *rt2x00pci = (struct rt2x00_pci*)ring-dev; - struct net_device *net_dev = pci_get_drvdata(rt2x00pci-pci_dev); - struct sk_buff *skb; + struct data_ring*ring = (struct data_ring*)data; No need for a cast here. In C you can cast from any struct pointer to void pointer and back without problems. @@ -784,8 +782,8 @@ static void rt2400pci_rxdone(void *data) { struct data_ring*ring = (struct data_ring*)data; No need for casting. @@ -835,8 +834,8 @@ static void rt2400pci_txdone(void *data) { struct data_ring*ring = (struct data_ring*)data; No need for casting. diff -uprN wireless-dev-rt2x00/drivers/net/wireless/d80211/rt2x00/rt2500pci.c wireless-dev-rt2x00-patch/drivers/net/wireless/d80211/rt2x00/rt2500pci.c --- wireless-dev-rt2x00/drivers/net/wireless/d80211/rt2x00/rt2500pci.c 2006-04-27 21:48:21.0 +0200 +++ wireless-dev-rt2x00-patch/drivers/net/wireless/d80211/rt2x00/rt2500pci.c 2006-04-27 21:49:08.0 +0200 @@ -834,10 +834,8 @@ rt2500pci_write_tx_desc( static void rt2500pci_beacondone(void *data) { - struct data_ring*ring = (struct data_ring*)data; - struct rt2x00_pci *rt2x00pci = (struct rt2x00_pci*)ring-dev; - struct net_device *net_dev = pci_get_drvdata(rt2x00pci-pci_dev); - struct sk_buff *skb; + struct data_ring*ring = (struct data_ring*)data; No need for casting. Many more of them. I guess you could just do a fgrep for *ring = (struct data_ring*)data; Regards Ingo Oeser - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: IP1000 gigabit nic driver
Hi David, David Gómez wrote: On Apr 28 at 01:58:04, Pekka Enberg wrote: Needs some serious coding style cleanup and conversion to proper 2.6 APIs for starters. Ok, i could take care of that, and it's a good way of getting my hands dirty with kernel programming ;). David, if it's ok to you i'll do the cleanup thing. Have fun! Great that you do this. What about 2.4/2.2 code? It's supposed to stay for compatibility or it should be removed before submitting? Usually it should be removed. The way to remove 2.4/2.2. code is by reimplementation of 2.6-APIs in seperate files and headers and not submitting these into latest kernel. Keep these somewhere else (e.g. a project web site). That way your drivers ALWAYS work with latest kernels and you notice breakage of backward compatiblity quite easily. If maintaining these parts becomes a pain with no gain, you can simply stop providing these yourself. #ifdef KERNEL_VERSON stuff in submitted drivers is generally not acceptable. Since it is hard to test these parts. I ported some off-tree drivers from 2.2 to 2.4. using this technique and it works good, reduces maintainence burden and keeps your driver current to latest APIs automatically. Regards Ingo Oeser - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Van Jacobson's net channels and real-time
Hi Dave, On Sunday, 23. April 2006 07:56, David S. Miller wrote: If cacheline bouncing because of the shared filled_entries becomes an issue, you are receiving or sending a lot. Cacheline bouncing is the core issue being addressed by this data structure, so we really can't consider your idea seriously. Ok, I can see it now more clearly. Many thanks for clearing that up in the other replies. I had a major misunderstanding there. I've just got an off-by-one error, no need to wreck the entire data structure just to solve that :-) Yes, you are right. But even then I can still implement the reserve/commit once you provide the helpers for producer_space and consumer_space. Regards Ingo Oeser - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Van Jacobson's net channels and real-time
Hi Jörn, On Saturday, 22. April 2006 13:48, Jörn Engel wrote: Unless I completely misunderstand something, one of the main points of the netchannels if to have *zero* fields written to by both producer and consumer. Hmm, for me the main point was to keep the complete processing of a single packet within one CPU/Core where this is a non-issue. Receiving and sending a lot can be expected to be the common case, so taking a performance hit in this case is hardly a good idea. There is no hit. If you receive/send in bursts you can simply aggregate them until a certain queueing threshold. The queue design outlined can split the queueing in reserve and commit stages, where the producer can be told how much in can produce and the consumer is told how much it can consume. Within their areas the producer and consumer can freely move around. So this is not exactly a queue, but a dynamic double buffer :-) So maybe doing queueing with the classic head/tail variant is better here, but the other variant might replace it without problems and allows for some nice improvements. Regards Ingo Oeser - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Van Jacobson's net channels and real-time
On Saturday, 22. April 2006 15:49, Jörn Engel wrote: That was another main point, yes. And the endpoints should be as little burden on the bottlenecks as possible. One bottleneck is the receive interrupt, which shouldn't wait for cachelines from other cpus too much. Thats right. This will be made a non issue with early demuxing on the NIC and MSI (or was it MSI-X?) which will select the right CPU based on hardware channels. In the meantime I would reduce the effects with only committing on full buffer or on leaving the interrupt handler. This would be ok, because here you have to wakeup the process anyway on full buffer and if it slept because of empty buffer. You loose only, if your application didn't sleep yet and you need to leave the interrupt handler because there is no work anymore. In this case the atomic_add would be significant. All this is quite similiar to now we do page_vec stuff in mm/ already. Regards Ingo Oeser - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Van Jacobson's net channels and real-time
Hi David, nice to see you getting started with it. I'm not sure about the queue logic there. 1867 /* Caller must have exclusive producer access to the netchannel. */ 1868 int netchannel_enqueue(struct netchannel *np, struct netchannel_buftrailer *bp) 1869 { 1870unsigned long tail; 1871 1872tail = np-netchan_tail; 1873if (tail == np-netchan_head) 1874return -ENOMEM; This looks wrong, since empty and full are the same condition in your case. 1891 struct netchannel_buftrailer *__netchannel_dequeue(struct netchannel *np) 1892 { 1893unsigned long head = np-netchan_head; 1894struct netchannel_buftrailer *bp = np-netchan_queue[head]; 1895 1896BUG_ON(np-netchan_tail == head); See? What about sth. like struct netchannel { /* This is only read/written by the writer (producer) */ unsigned long write_ptr; struct netchannel_buftrailer *netchan_queue[NET_CHANNEL_ENTRIES]; /* This is modified by both */ atomic_t filled_entries; /* cache_line_align this? */ /* This is only read/written by the reader (consumer) */ unsigned long read_ptr; } This would prevent this bug from the beginning and let us still use the full queue size. If cacheline bouncing because of the shared filled_entries becomes an issue, you are receiving or sending a lot. Then you can enqueue and dequeue multiple and commit the counts later. To be done with a atomic_read, atomic_add and atomic_sub on filled_entries. Maybe even cheaper with local_t instead of atomic_t later on. But I guess the cacheline bouncing will be a non-issue, since the whole point of netchannels was to keep traffic as local to a cpu as possible, right? Would you like to see a sample patch relative to your tree, to show you what I mean? Regards Ingo Oeser - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/10] [IOAT] I/OAT patches repost
David S. Miller wrote: The first thing an application is going to do is touch that data. So I think it's very important to prewarm the caches and the only straightforward way I know of to always warm up the correct cpu's caches is copy_to_user(). Hmm, what if the application is sth. like a MPEG demultiplexer? There you don't like to look at everything and excplicitly ignore received data[1]. Yes, I know this is usually done with hardware demuxers and filters, but the principle might apply to other applications as well, for which no hardware solutions exist. Regards Ingo Oeser [1] which you cannot ignore properly with Linux yet. - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [TCP]: Fix truesize underflow
Hi Herbert, Herbert Xu wrote: I've copied the code you used in tso_fragment which should work here. I'm happy to see, that this got resolved and this is a nice minimalistic fix for -stable. But shouldn't we put this kind of hairy manipulation into some nice functions? Driver writers were already confused by all that size, len and truesize stuff, as this bug showed. Regards Ingo Oeser - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] deinline a few large functions in vlan code v2
Hi Dave, On Wednesday, 12. April 2006 19:18, Dave Dillow wrote: you've left the spin_locks in, and have more #ifdefs. Ok, I can refactor your driver to even remove this and reduce it to exaxtly two ifdef sections for your driver. Acceptable? Regardless, I remain opposed to this particular instance of bloat busting. While both patches have improved in style, they remove a useful feature and make the code less clean, for no net gain. s/useful feature/unreachable code/g On SMP FC4, typhoon.ko has a text size of 68330, so you need to cut 2794 bytes to see an actual difference in memory usage for a module. Non-SMP it is 67741, so there you only need to cut 2205 bytes to get a win. textdata bss dec hex filename 62079 973 4 63056f650 no-vlan/drivers/net/typhoon.o 62654 973 4 63631f88f vlan/drivers/net/typhoon.o with allyesconfig (minus CONFIG_INFO) and my patches applied. So maybe the uninlining is enough. Gain after this is just 575 bytes here. Regards Ingo Oeser - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFD][PATCH] typhoon and core sample for folding away VLAN stuff (was: Re: [PATCH] deinline a few large functions in vlan code v2)
Hi Denis, here is a sample patch for the vlan core and API plus typhoon driver converted as example. Just so you can see, what I meant with No #if in control flow code. I couldn't resist cleaning up the vlan core, while I'm at it. Of course I can seperate this, if you want the pure unilining stuff only. So I hope this style is clear enough that you can do the rest of the conversion without any problems. Dave Dillow: Is this style clean enough to have it in your driver? This kind of changes are important, because bloat creeps in byte by byte of unused features. So I really appreciate your work here Denis. Regards Ingo Oeser diff --git a/drivers/net/typhoon.c b/drivers/net/typhoon.c index c1ce87a..aab24b8 100644 --- a/drivers/net/typhoon.c +++ b/drivers/net/typhoon.c @@ -285,7 +285,9 @@ struct typhoon { struct pci_dev *pdev; struct net_device * dev; spinlock_t state_lock; +#if defined(CONFIG_VLAN_8021Q) || defined(CONFIG_VLAN_8021Q_MODULE) struct vlan_group * vlgrp; +#endif struct basic_ring rxHiRing; struct basic_ring rxBuffRing; struct rxbuff_ent rxbuffers[RXENT_ENTRIES]; @@ -350,6 +352,19 @@ enum state_values { #define TSO_OFFLOAD_ON 0 #endif + +#if defined(CONFIG_VLAN_8021Q) || defined(CONFIG_VLAN_8021Q_MODULE) +static inline struct vlan_group *typhoon_get_vlgrp(struct typhoon *tp) +{ + return tp-vlgrp; +} +#else +static inline struct vlan_group *typhoon_get_vlgrp(struct typhoon *tp) +{ + return NULL; +} +#endif + static inline void typhoon_inc_index(u32 *index, const int count, const int num_entries) { @@ -707,6 +722,7 @@ out: return err; } +#if defined(CONFIG_VLAN_8021Q) || defined(CONFIG_VLAN_8021Q_MODULE) static void typhoon_vlan_rx_register(struct net_device *dev, struct vlan_group *grp) { @@ -754,6 +770,12 @@ typhoon_vlan_rx_kill_vid(struct net_devi tp-vlgrp-vlan_devices[vid] = NULL; spin_unlock_bh(tp-state_lock); } +#else + +#define typhoon_vlan_rx_register NULL +#define typhoon_vlan_rx_kill_vid NULL + +#endif static inline void typhoon_tso_fill(struct sk_buff *skb, struct transmit_ring *txRing, @@ -1686,6 +1708,7 @@ typhoon_rx(struct typhoon *tp, struct ba struct rx_desc *rx; struct sk_buff *skb, *new_skb; struct rxbuff_ent *rxb; + struct vlan_group *vlgrp; dma_addr_t dma_addr; u32 local_ready; u32 rxaddr; @@ -1745,8 +1768,9 @@ typhoon_rx(struct typhoon *tp, struct ba new_skb-ip_summed = CHECKSUM_NONE; spin_lock(tp-state_lock); - if(tp-vlgrp != NULL rx-rxStatus TYPHOON_RX_VLAN) - vlan_hwaccel_receive_skb(new_skb, tp-vlgrp, + vlgrp = typhoon_get_vlgrp(tp); + if(vlgrp != NULL rx-rxStatus TYPHOON_RX_VLAN) + vlan_hwaccel_receive_skb(new_skb, vlgrp, ntohl(rx-vlanTag) 0x); else netif_receive_skb(new_skb); @@ -2233,7 +2257,7 @@ typhoon_suspend(struct pci_dev *pdev, pm return 0; spin_lock_bh(tp-state_lock); - if(tp-vlgrp tp-wol_events TYPHOON_WAKE_MAGIC_PKT) { + if(typhoon_get_vlgrp(tp) tp-wol_events TYPHOON_WAKE_MAGIC_PKT) { spin_unlock_bh(tp-state_lock); printk(KERN_ERR %s: cannot do WAKE_MAGIC with VLANS\n, dev-name); diff --git a/include/linux/if_vlan.h b/include/linux/if_vlan.h index eef0876..4ed541f 100644 --- a/include/linux/if_vlan.h +++ b/include/linux/if_vlan.h @@ -14,6 +14,8 @@ #define _LINUX_IF_VLAN_H_ #ifdef __KERNEL__ +#define HAVE_VLAN_PUT_TAG +#define HAVE_VLAN_GET_TAG /* externally defined structs */ struct vlan_group; @@ -119,79 +121,29 @@ struct vlan_dev_info { struct net_device_stats dev_stats; /* Device stats (rx-bytes, tx-pkts, etc...) */ }; -#define VLAN_DEV_INFO(x) ((struct vlan_dev_info *)(x-priv)) - -/* inline functions */ - -static inline struct net_device_stats *vlan_dev_get_stats(struct net_device *dev) -{ - return (VLAN_DEV_INFO(dev)-dev_stats); -} - -static inline __u32 vlan_get_ingress_priority(struct net_device *dev, - unsigned short vlan_tag) -{ - struct vlan_dev_info *vip = VLAN_DEV_INFO(dev); - - return vip-ingress_priority_map[(vlan_tag 13) 0x7]; -} - /* VLAN tx hw acceleration helpers. */ struct vlan_skb_tx_cookie { u32 magic; u32 vlan_tag; }; -#define VLAN_TX_COOKIE_MAGIC 0x564c414e /* VLAN in ascii. */ #define VLAN_TX_SKB_CB(__skb) ((struct vlan_skb_tx_cookie *)((__skb)-cb[0])) -#define vlan_tx_tag_present(__skb) \ - (VLAN_TX_SKB_CB(__skb)-magic == VLAN_TX_COOKIE_MAGIC) + #define vlan_tx_tag_get(__skb) (VLAN_TX_SKB_CB(__skb)-vlan_tag) -/* VLAN rx hw acceleration helper
Re: [PATCH] forcedeth: suggested cleanups
Hi Manfred, Manfred Spraul wrote: I think the patch should wait until 0.57 is merged: There are 4 patches on their way to Jeff. These patches contain several bugfixes, they should have the higher priority. Fine with me. Will resubmit after next week. Apart from that: looks good. Many thanks for your review. Regards Ingo Oeser - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] deinline a few large functions in vlan code v2
Hi Denis, Denis Vlasenko wrote: +#if defined(CONFIG_VLAN_8021Q) || defined(CONFIG_VLAN_8021Q_MODULE) if(vlan_tx_tag_present(skb)) { first_txd-processFlags |= TYPHOON_TX_PF_INSERT_VLAN | TYPHOON_TX_PF_VLAN_PRIORITY; @@ -844,6 +849,7 @@ typhoon_start_tx(struct sk_buff *skb, st cpu_to_le32(htons(vlan_tx_tag_get(skb)) TYPHOON_TX_PF_VLAN_TAG_SHIFT); } +#endif Wouldn't it be much easier to just do #if defined(CONFIG_VLAN_8021Q) || defined(CONFIG_VLAN_8021Q_MODULE) static inline int vlan_tx_tag_present(...) { /** get VLAN tag */ } #else static inline int vlan_tx_tag_present(...) {return 0;} #endif in some header file? Similiar in typhoon.c: #if defined(CONFIG_VLAN_8021Q) || defined(CONFIG_VLAN_8021Q_MODULE) static inline has_vlan_group(...) { /* get VLAN group */ } #else static inline has_vlan_group(...) {return 0;} #endif With this and similiar changes in the drivers, your patch might be less intrusive and thus more acceptable to maintainers. Just let the compiler remove the extra code with constant folding and dead code elemination. The result will be even cleaner code, I think. What do you think? Regards Ingo Oeser - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] deinline a few large functions in vlan code v2
Hi Denis, Denis Vlasenko wrote: On Tuesday 11 April 2006 12:49, Ingo Oeser wrote: #if defined(CONFIG_VLAN_8021Q) || defined(CONFIG_VLAN_8021Q_MODULE) static inline has_vlan_group(...) { /* get VLAN group */ } #else static inline has_vlan_group(...) {return 0;} #endif With this and similiar changes in the drivers, your patch might be less intrusive and thus more acceptable to maintainers. Just let the compiler remove the extra code with constant folding and dead code elemination. The result will be even cleaner code, I think. What do you think? I thought more of introducing functions, which just fold away all the if () blocks in normal code paths, which you wrapped into #if here. I don't think people have problems, if you #ifdef out complete functions, linear setup code or structure members. People seem to have more problems with #ifdef in control flow code, because there the condition is nothing else but a compile time constant (the CONFIG_FOO value) which should be expressed as such. Instead of #ifdef CONFIG_FOO if (condition) { } #endif just do #ifdef CONFIG_FOO #define foo 1 #else #define foo 0 #endif if (foo condition) { } Just do nothing or return a compile time constant in those inlines. Addresses of these functions are stored into netdevice members: @@ -2549,8 +2559,10 @@ typhoon_init_one(struct pci_dev *pdev, c dev-watchdog_timeo = TX_TIMEOUT; dev-get_stats = typhoon_get_stats; dev-set_mac_address= typhoon_set_mac_address; +#if defined(CONFIG_VLAN_8021Q) || defined(CONFIG_VLAN_8021Q_MODULE) dev-vlan_rx_register = typhoon_vlan_rx_register; dev-vlan_rx_kill_vid = typhoon_vlan_rx_kill_vid; +#endif Even empty inline would not be optimized out to nothing here. No problem, just optimize out the assignment itself: #if defined(CONFIG_VLAN_8021Q) || defined(CONFIG_VLAN_8021Q_MODULE) static inline void typhoon_setup_vlan_hooks(struct netdev *dev) { dev-vlan_rx_register = typhoon_vlan_rx_register; dev-vlan_rx_kill_vid = typhoon_vlan_rx_kill_vid; } #else static inline void typhoon_setup_vlan_hooks(struct netdev *dev) { (void)dev; } #endif The whole linux/if_vlan.h stuff misses this style of code. There is simply no fallback code for CONFIG_VLAN_8021Q not being defined. Regards Ingo Oeser - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] fix multicast frame handling
Hi Vlad, Vlad Drukker wrote: diff --git a/net/core/dev.c b/net/core/dev.c index 434220d..a351687 100644 --- a/net/core/dev.c +++ b/net/core/dev.c @@ -1614,6 +1614,8 @@ static __inline__ int handle_bridge(stru struct net_bridge_port *port; if ((*pskb)-pkt_type == PACKET_LOOPBACK || + ((*pskb)-dev-priv_flags IFF_MASTER_8023AD + (*pskb)-protocol == __constant_htons(ETH_P_SLOW)) || (port = rcu_dereference((*pskb)-dev-br_port)) == NULL) return 0; Please use htons(ETH_P_SLOW), since the compiler will constant fold this and it is more readable this way. Regards Ingo Oeser - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] forcedeth: suggested cleanups
From: Ingo Oeser [EMAIL PROTECTED] general: - endian annotation of the ring descriptors nv_getlen(): - use htons() instead of __constant_htons() to improvde readability and let the compiler constant fold it. nv_rx_process(): - use a real for() loop in processing instead of goto and break - consolidate rx_errors increment - count detected rx_length_errors Signed-off-by: Ingo Oeser [EMAIL PROTECTED] --- Hi there, here are some suggested cleanups, which compiled without problems using allyesconfig on the latest net-2.6 git tree from David S. Miller. Please review and apply, if convienient. Thanks and Regards Ingo Oeser forcedeth.c | 59 ++- 1 file changed, 26 insertions(+), 33 deletions(-) diff --git a/drivers/net/forcedeth.c b/drivers/net/forcedeth.c index 7627a75..0d2866b 100644 --- a/drivers/net/forcedeth.c +++ b/drivers/net/forcedeth.c @@ -328,17 +328,18 @@ enum { NvRegMSIXIrqStatus = 0x3f0, }; -/* Big endian: should work, but is untested */ +/* Big endian: should work, but is untested. + * So give arch maintainers a hint here. -ioe */ struct ring_desc { - u32 PacketBuffer; - u32 FlagLen; + __le32 PacketBuffer; + __le32 FlagLen; }; struct ring_desc_ex { - u32 PacketBufferHigh; - u32 PacketBufferLow; - u32 TxVlan; - u32 FlagLen; + __le32 PacketBufferHigh; + __le32 PacketBufferLow; + __le32 TxVlan; + __le32 FlagLen; }; typedef union _ring_type { @@ -1403,7 +1404,7 @@ static int nv_getlen(struct net_device * int protolen; /* length as stored in the proto field */ /* 1) calculate len according to header */ - if ( ((struct vlan_ethhdr *)packet)-h_vlan_proto == __constant_htons(ETH_P_8021Q)) { + if ( ((struct vlan_ethhdr *)packet)-h_vlan_proto == htons(ETH_P_8021Q)) { protolen = ntohs( ((struct vlan_ethhdr *)packet)-h_vlan_encapsulated_proto ); hdrlen = VLAN_HLEN; } else { @@ -1453,12 +1454,10 @@ static void nv_rx_process(struct net_dev u32 vlanflags = 0; - for (;;) { + for (; np-cur_rx - np-refill_rx RX_RING; np-cur_rx++) { struct sk_buff *skb; int len; int i; - if (np-cur_rx - np-refill_rx = RX_RING) - break; /* we scanned the whole ring - do not continue */ i = np-cur_rx % RX_RING; if (np-desc_ver == DESC_VER_1 || np-desc_ver == DESC_VER_2) { @@ -1498,33 +1497,29 @@ static void nv_rx_process(struct net_dev /* look at what we actually got: */ if (np-desc_ver == DESC_VER_1) { if (!(Flags NV_RX_DESCRIPTORVALID)) - goto next_pkt; + continue; if (Flags NV_RX_ERROR) { if (Flags NV_RX_MISSEDFRAME) { np-stats.rx_missed_errors++; - np-stats.rx_errors++; - goto next_pkt; + goto error_pkt; } if (Flags (NV_RX_ERROR1|NV_RX_ERROR2|NV_RX_ERROR3)) { - np-stats.rx_errors++; - goto next_pkt; + goto error_pkt; } if (Flags NV_RX_CRCERR) { np-stats.rx_crc_errors++; - np-stats.rx_errors++; - goto next_pkt; + goto error_pkt; } if (Flags NV_RX_OVERFLOW) { np-stats.rx_over_errors++; - np-stats.rx_errors++; - goto next_pkt; + goto error_pkt; } if (Flags NV_RX_ERROR4) { len = nv_getlen(dev, np-rx_skbuff[i]-data, len); if (len 0) { - np-stats.rx_errors++; - goto next_pkt; + np-stats.rx_length_errors++; + goto error_pkt; } } /* framing errors are soft errors. */ @@ -1536,28 +1531,25 @@ static void nv_rx_process(struct net_dev
Re: [PATCH] softmac: return -EAGAIN from getscan while scanning
Hi, Johannes Berg wrote: Below patch was developed after discussion with Daniel Drake who mentioned to me that wireless tools expect an EAGAIN return from getscan so that they can wait for the scan to finish before printing out the results. And why do you implement it across two files? I would suggest folding it simply into its sole caller: ieee80211softmac_wx_get_scan_results() in net/ieee80211/softmac/ieee80211softmac_wx.c That should reduce kernel text size a bit. Regards Ingo Oeser - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/9] [I/OAT] DMA memcpy subsystem
Kumar Gala wrote: On Mar 30, 2006, at 12:36 PM, Andrew Grover wrote: Wellit's true they're useful for debugging but I would put them in the category of system statistics that shouldn't go in debugfs. I think they are like /proc/interrupts' interrupt counts or the TX/RX stats reported by ifconfig. Fair, but wouldn't it be better to have the association per client. Maybe leave the one as a summary and have a dir per client with similar stats that are for each client and add a per channel summary at the top level as well. Such level of detail really belongs to debugging, IMHO. I think, it would suffer to say, how many channels are in use. So you can answer the question, whether your customers are actually using this experimental technology. If you want more, let them mount debugfs. If it becomes really important, we can revisit this later. Thats the advantage of files under debugfs not being stable API in any way. BTW: What is the actual frequency, at which such counters will be incremented? Regards Ingo Oeser - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [e1000 debug] KERNEL: assertion (!sk_forward_alloc) failed...
Hi Jesse, More datapoints. First of all, I don't see the problem, so this is an exclusion data point. Machine is up 1 day, 19:02 I use 2.6.16 and I'm NBOT running at Gigabit speed. (just couldn't get e100 cards anymore, they are not sold anymore here) Version: vendor 00:aa:00, model 56 rev 0 Services: I'm routing, run IPsec, do firewalling/nat with iptables, do PPPoE on this machine but all not on that interface. The card is exposed to the local LAN interface. [4294671.426000] e1000: eth0: e1000_probe: Intel(R) PRO/1000 Network Connection [4294671.447000] e100: eth1: e100_probe: addr 0xe314, irq 10, MAC addr 00:D0:B7:XX:XX:XX [4294671.447000] eth2: VIA Rhine II at 0xe3142000, 00:0f:ea:XX:XX:XX, IRQ 11. [4294671.448000] eth2: MII PHY found at address 1, status 0x786d advertising 05e1 Link 41e1. [4294671.448000] forcedeth.c: Reverse Engineered nForce ethernet driver. Version 0.49. [4294679.125000] e1000: eth0: e1000_watchdog_task: NIC Link is Up 100 Mbps Full Duplex [4294679.165000] e100: eth1: e100_watchdog: link up, 10Mbps, half-duplex [4294679.201000] eth2: link up, 100Mbps, full-duplex, lpa 0x41E1 lspci with info for this card. :00:0c.0 Ethernet controller: Intel Corp. 82541GI/PI Gigabit Ethernet Controller Subsystem: Intel Corp. PRO/1000 MT Desktop Adapter Control: I/O+ Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR- FastB2B- Status: Cap+ 66MHz+ UDF- FastB2B- ParErr- DEVSEL=medium TAbort- TAbort- MAbort- SERR- PERR- Latency: 32 (63750ns min), Cache Line Size: 0x08 (32 bytes) Interrupt: pin A routed to IRQ 11 Region 0: Memory at e310 (32-bit, non-prefetchable) [size=128K] Region 1: Memory at e312 (32-bit, non-prefetchable) [size=128K] Region 2: I/O ports at d400 [size=64] Expansion ROM at 2010 [disabled] [size=128K] Capabilities: [dc] Power Management version 2 Flags: PMEClk- DSI+ D1- D2- AuxCurrent=0mA PME(D0+,D1-,D2-,D3hot+,D3cold+) Status: D0 PME-Enable- DSel=0 DScale=1 PME- Capabilities: [e4] PCI-X non-bridge device. Command: DPERE- ERO+ RBC=0 OST=0 Status: Bus=0 Dev=0 Func=0 64bit- 133MHz- SCD- USC-, DC=simple, DMMRBC=2, DMOST=0, DMCRS=0, RSCEM- Capabilities: [f0] Message Signalled Interrupts: 64bit+ Queue=0/0 Enable- Address: Data: ip -s -s link dev eth0 1: eth0: BROADCAST,MULTICAST,UP mtu 1500 qdisc pfifo_fast qlen 1000 link/ether 00:0e:0c:XX:XX:XX brd ff:ff:ff:ff:ff:ff RX: bytes packets errors dropped overrun mcast 648259157 1081155 0 0 0 115 RX errors: length crc frame fifomissed 00 0 0 0 TX: bytes packets errors dropped carrier collsns 393218241 933436 0 0 0 0 TX errors: aborted fifowindow heartbeat 00 0 0 My config is attached, more data on request. I can play with parameters, but cannot test patches. Regards Ingo Oeser # # Automatically generated make config: don't edit # Linux kernel version: 2.6.16 # Thu Mar 23 15:29:59 2006 # CONFIG_X86_32=y CONFIG_SEMAPHORE_SLEEPERS=y CONFIG_X86=y CONFIG_MMU=y CONFIG_GENERIC_ISA_DMA=y CONFIG_GENERIC_IOMAP=y CONFIG_ARCH_MAY_HAVE_PC_FDC=y CONFIG_DMI=y # # Code maturity level options # CONFIG_EXPERIMENTAL=y CONFIG_BROKEN_ON_SMP=y CONFIG_INIT_ENV_ARG_LIMIT=32 # # General setup # CONFIG_LOCALVERSION= CONFIG_LOCALVERSION_AUTO=y CONFIG_SWAP=y CONFIG_SYSVIPC=y # CONFIG_POSIX_MQUEUE is not set # CONFIG_BSD_PROCESS_ACCT is not set CONFIG_SYSCTL=y # CONFIG_AUDIT is not set CONFIG_IKCONFIG=y CONFIG_IKCONFIG_PROC=y CONFIG_INITRAMFS_SOURCE= CONFIG_UID16=y # CONFIG_VM86 is not set # CONFIG_CC_OPTIMIZE_FOR_SIZE is not set CONFIG_EMBEDDED=y CONFIG_KALLSYMS=y # CONFIG_KALLSYMS_ALL is not set # CONFIG_KALLSYMS_EXTRA_PASS is not set CONFIG_HOTPLUG=y CONFIG_PRINTK=y CONFIG_BUG=y CONFIG_ELF_CORE=y CONFIG_BASE_FULL=y CONFIG_FUTEX=y CONFIG_EPOLL=y CONFIG_SHMEM=y CONFIG_CC_ALIGN_FUNCTIONS=0 CONFIG_CC_ALIGN_LABELS=0 CONFIG_CC_ALIGN_LOOPS=0 CONFIG_CC_ALIGN_JUMPS=0 CONFIG_SLAB=y # CONFIG_TINY_SHMEM is not set CONFIG_BASE_SMALL=0 # CONFIG_SLOB is not set # # Loadable module support # # CONFIG_MODULES is not set # # Block layer # # CONFIG_LBD is not set # # IO Schedulers # CONFIG_IOSCHED_NOOP=y CONFIG_IOSCHED_AS=y CONFIG_IOSCHED_DEADLINE=y CONFIG_IOSCHED_CFQ=y CONFIG_DEFAULT_AS=y # CONFIG_DEFAULT_DEADLINE is not set # CONFIG_DEFAULT_CFQ is not set # CONFIG_DEFAULT_NOOP is not set CONFIG_DEFAULT_IOSCHED=anticipatory # # Processor type and features # CONFIG_X86_PC=y # CONFIG_X86_ELAN is not set # CONFIG_X86_VOYAGER is not set # CONFIG_X86_NUMAQ is not set # CONFIG_X86_SUMMIT is not set # CONFIG_X86_BIGSMP is not set # CONFIG_X86_VISWS is not set # CONFIG_X86_GENERICARCH is not set # CONFIG_X86_ES7000 is not set # CONFIG_M386 is not set
Re: [e1000 debug] KERNEL: assertion (!sk_forward_alloc) failed...
Hi, Herbert Xu wrote: On Fri, Mar 31, 2006 at 01:35:40AM -0800, David S. Miller wrote: He does not have TSO enabled, e1000 disables TSO when on a link speed slower than gigabit. dmesg|grep eth0 [4294671.426000] e1000: eth0: e1000_probe: Intel(R) PRO/1000 Network Connection [4294679.125000] e1000: eth0: e1000_watchdog_task: NIC Link is Up 100 Mbps Full Duplex # ethtool -k eth0 Offload parameters for eth0: rx-checksumming: on tx-checksumming: on scatter-gather: on tcp segmentation offload: on So this theory doesn't seem to hold :-( Indeed. But I think that only happens on PCI Express and I don't think Ingo is using PCI Express. Right. PCI-Express is not available in this machine. Maybe the traffic is not enough to trigger it. External connect is just a 6MBit DSL. Regards Ingo Oeser - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] scm: fold __scm_send() into scm_send()
From: Ingo Oeser [EMAIL PROTECTED] Fold __scm_send() into scm_send() and remove that interface completly from the kernel. Signed-off-by: Ingo Oeser [EMAIL PROTECTED] --- Inspired by the patch to inline scm_send() I did the next logical step :-) Regards Ingo Oeser diff --git a/include/net/scm.h b/include/net/scm.h index eb44e5a..ec8b891 100644 --- a/include/net/scm.h +++ b/include/net/scm.h @@ -26,11 +26,9 @@ struct scm_cookie extern void scm_detach_fds(struct msghdr *msg, struct scm_cookie *scm); extern void scm_detach_fds_compat(struct msghdr *msg, struct scm_cookie *scm); -extern int __scm_send(struct socket *sock, struct msghdr *msg, struct scm_cookie *scm); extern void __scm_destroy(struct scm_cookie *scm); extern struct scm_fp_list * scm_fp_dup(struct scm_fp_list *fpl); -extern int scm_send(struct socket *sock, struct msghdr *msg, - struct scm_cookie *scm); +extern int scm_send(struct socket *sock, struct msghdr *msg, struct scm_cookie *scm); extern void scm_recv(struct socket *sock, struct msghdr *msg, struct scm_cookie *scm, int flags); diff --git a/net/core/scm.c b/net/core/scm.c index b6dee90..6adbe60 100644 --- a/net/core/scm.c +++ b/net/core/scm.c @@ -110,10 +110,21 @@ void __scm_destroy(struct scm_cookie *sc } } -int __scm_send(struct socket *sock, struct msghdr *msg, struct scm_cookie *p) +int scm_send(struct socket *sock, struct msghdr *msg, struct scm_cookie *scm) { struct cmsghdr *cmsg; int err; + struct task_struct *tsk = current; + scm-creds = (struct ucred) { + .uid = tsk-uid, + .gid = tsk-gid, + .pid = tsk-tgid + }; + scm-fp = NULL; + scm-sid = security_sk_sid(sock-sk, NULL, 0); + scm-seq = 0; + if (msg-msg_controllen = 0) + return 0; for (cmsg = CMSG_FIRSTHDR(msg); cmsg; cmsg = CMSG_NXTHDR(msg, cmsg)) { @@ -136,15 +147,15 @@ int __scm_send(struct socket *sock, stru switch (cmsg-cmsg_type) { case SCM_RIGHTS: - err=scm_fp_copy(cmsg, p-fp); + err=scm_fp_copy(cmsg, scm-fp); if (err0) goto error; break; case SCM_CREDENTIALS: if (cmsg-cmsg_len != CMSG_LEN(sizeof(struct ucred))) goto error; - memcpy(p-creds, CMSG_DATA(cmsg), sizeof(struct ucred)); - err = scm_check_creds(p-creds); + memcpy(scm-creds, CMSG_DATA(cmsg), sizeof(struct ucred)); + err = scm_check_creds(scm-creds); if (err) goto error; break; @@ -153,15 +164,15 @@ int __scm_send(struct socket *sock, stru } } - if (p-fp !p-fp-count) + if (scm-fp !scm-fp-count) { - kfree(p-fp); - p-fp = NULL; + kfree(scm-fp); + scm-fp = NULL; } return 0; error: - scm_destroy(p); + scm_destroy(scm); return err; } @@ -284,22 +295,6 @@ struct scm_fp_list *scm_fp_dup(struct sc return new_fpl; } -int scm_send(struct socket *sock, struct msghdr *msg, struct scm_cookie *scm) -{ - struct task_struct *p = current; - scm-creds = (struct ucred) { - .uid = p-uid, - .gid = p-gid, - .pid = p-tgid - }; - scm-fp = NULL; - scm-sid = security_sk_sid(sock-sk, NULL, 0); - scm-seq = 0; - if (msg-msg_controllen = 0) - return 0; - return __scm_send(sock, msg, scm); -} - void scm_recv(struct socket *sock, struct msghdr *msg, struct scm_cookie *scm, int flags) { @@ -332,7 +326,6 @@ void scm_recv(struct socket *sock, struc } EXPORT_SYMBOL(__scm_destroy); -EXPORT_SYMBOL(__scm_send); EXPORT_SYMBOL(scm_send); EXPORT_SYMBOL(scm_recv); EXPORT_SYMBOL(put_cmsg); - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] Nearly complete kzalloc cleanup for net/ipv6
From: Ingo Oeser [EMAIL PROTECTED] Stupidly use kzalloc() instead of kmalloc()/memset() everywhere where this is possible in net/ipv6/*.c . Signed-off-by: Ingo Oeser [EMAIL PROTECTED] --- Hi, I'm going to refactor some of the simple cases to reduce code duplication which will depend on this patch. net/ipv6/reassambly.c will get a separate patch, since I refactored it a little bit. The netfilter part is NOT included, because Harald should see these, too. Patch is against net-2.6.17 Regards Ingo Oeser diff --git a/net/ipv6/ah6.c b/net/ipv6/ah6.c index 8ff7147..b876a10 100644 --- a/net/ipv6/ah6.c +++ b/net/ipv6/ah6.c @@ -354,12 +354,10 @@ static int ah6_init_state(struct xfrm_st if (x-encap) goto error; - ahp = kmalloc(sizeof(*ahp), GFP_KERNEL); + ahp = kzalloc(sizeof(*ahp), GFP_KERNEL); if (ahp == NULL) return -ENOMEM; - memset(ahp, 0, sizeof(*ahp)); - ahp-key = x-aalg-alg_key; ahp-key_len = (x-aalg-alg_key_len+7)/8; ahp-tfm = crypto_alloc_tfm(x-aalg-alg_name, 0); diff --git a/net/ipv6/anycast.c b/net/ipv6/anycast.c index 840a33d..39ec528 100644 --- a/net/ipv6/anycast.c +++ b/net/ipv6/anycast.c @@ -308,7 +308,7 @@ int ipv6_dev_ac_inc(struct net_device *d * not found: create a new one. */ - aca = kmalloc(sizeof(struct ifacaddr6), GFP_ATOMIC); + aca = kzalloc(sizeof(struct ifacaddr6), GFP_ATOMIC); if (aca == NULL) { err = -ENOMEM; @@ -322,8 +322,6 @@ int ipv6_dev_ac_inc(struct net_device *d goto out; } - memset(aca, 0, sizeof(struct ifacaddr6)); - ipv6_addr_copy(aca-aca_addr, addr); aca-aca_idev = idev; aca-aca_rt = rt; @@ -550,7 +548,7 @@ static int ac6_seq_open(struct inode *in { struct seq_file *seq; int rc = -ENOMEM; - struct ac6_iter_state *s = kmalloc(sizeof(*s), GFP_KERNEL); + struct ac6_iter_state *s = kzalloc(sizeof(*s), GFP_KERNEL); if (!s) goto out; @@ -561,7 +559,6 @@ static int ac6_seq_open(struct inode *in seq = file-private_data; seq-private = s; - memset(s, 0, sizeof(*s)); out: return rc; out_kfree: diff --git a/net/ipv6/esp6.c b/net/ipv6/esp6.c index aa7f100..3dcaac7 100644 --- a/net/ipv6/esp6.c +++ b/net/ipv6/esp6.c @@ -305,12 +305,10 @@ static int esp6_init_state(struct xfrm_s if (x-encap) goto error; - esp = kmalloc(sizeof(*esp), GFP_KERNEL); + esp = kzalloc(sizeof(*esp), GFP_KERNEL); if (esp == NULL) return -ENOMEM; - memset(esp, 0, sizeof(*esp)); - if (x-aalg) { struct xfrm_algo_desc *aalg_desc; diff --git a/net/ipv6/ip6_flowlabel.c b/net/ipv6/ip6_flowlabel.c index 69cbe8a..f9ca639 100644 --- a/net/ipv6/ip6_flowlabel.c +++ b/net/ipv6/ip6_flowlabel.c @@ -287,10 +287,9 @@ fl_create(struct in6_flowlabel_req *freq int err; err = -ENOMEM; - fl = kmalloc(sizeof(*fl), GFP_KERNEL); + fl = kzalloc(sizeof(*fl), GFP_KERNEL); if (fl == NULL) goto done; - memset(fl, 0, sizeof(*fl)); olen = optlen - CMSG_ALIGN(sizeof(*freq)); if (olen 0) { @@ -663,7 +662,7 @@ static int ip6fl_seq_open(struct inode * { struct seq_file *seq; int rc = -ENOMEM; - struct ip6fl_iter_state *s = kmalloc(sizeof(*s), GFP_KERNEL); + struct ip6fl_iter_state *s = kzalloc(sizeof(*s), GFP_KERNEL); if (!s) goto out; @@ -674,7 +673,6 @@ static int ip6fl_seq_open(struct inode * seq = file-private_data; seq-private = s; - memset(s, 0, sizeof(*s)); out: return rc; out_kfree: diff --git a/net/ipv6/ipcomp6.c b/net/ipv6/ipcomp6.c index 3c7b324..028b636 100644 --- a/net/ipv6/ipcomp6.c +++ b/net/ipv6/ipcomp6.c @@ -428,11 +428,10 @@ static int ipcomp6_init_state(struct xfr goto out; err = -ENOMEM; - ipcd = kmalloc(sizeof(*ipcd), GFP_KERNEL); + ipcd = kzalloc(sizeof(*ipcd), GFP_KERNEL); if (!ipcd) goto out; - memset(ipcd, 0, sizeof(*ipcd)); x-props.header_len = 0; if (x-props.mode) x-props.header_len += sizeof(struct ipv6hdr); diff --git a/net/ipv6/mcast.c b/net/ipv6/mcast.c index 807c021..6e871af 100644 --- a/net/ipv6/mcast.c +++ b/net/ipv6/mcast.c @@ -767,10 +767,10 @@ static void mld_add_delrec(struct inet6_ * for deleted items allows change reports to use common code with * non-deleted or query-response MCA's. */ - pmc = kmalloc(sizeof(*pmc), GFP_ATOMIC); + pmc = kzalloc(sizeof(*pmc), GFP_ATOMIC); if (!pmc) return; - memset(pmc, 0, sizeof(*pmc)); + spin_lock_bh(im-mca_lock); spin_lock_init(pmc-mca_lock); pmc-idev = im-idev; @@ -893,7 +893,7 @@ int ipv6_dev_mc_inc(struct net_device
[PATCH] IPv6: Cleanup of net/ipv6/reassambly.c
From: Ingo Oeser [EMAIL PROTECTED] Two minor cleanups: 1. Using kzalloc() in fraq_alloc_queue() saves the memset() in ipv6_frag_create(). 2. Invert sense of if-statements to streamline code. Inverts the comment, too. Signed-off-by: Ingo Oeser [EMAIL PROTECTED] --- Hi, I first did the kzalloc cleanup, but then decided, that I can do this change, too. If you consider it worse, please tell me and I'll just do the kzalloc() one. Regards Ingo Oeser diff --git a/net/ipv6/reassembly.c b/net/ipv6/reassembly.c index 15e1456..6d8c9bb 100644 --- a/net/ipv6/reassembly.c +++ b/net/ipv6/reassembly.c @@ -203,7 +203,7 @@ static inline void frag_free_queue(struc static inline struct frag_queue *frag_alloc_queue(void) { - struct frag_queue *fq = kmalloc(sizeof(struct frag_queue), GFP_ATOMIC); + struct frag_queue *fq = kzalloc(sizeof(struct frag_queue), GFP_ATOMIC); if(!fq) return NULL; @@ -288,6 +288,7 @@ static void ip6_evictor(void) static void ip6_frag_expire(unsigned long data) { struct frag_queue *fq = (struct frag_queue *) data; + struct net_device *dev; spin_lock(fq-lock); @@ -299,22 +300,22 @@ static void ip6_frag_expire(unsigned lon IP6_INC_STATS_BH(IPSTATS_MIB_REASMTIMEOUT); IP6_INC_STATS_BH(IPSTATS_MIB_REASMFAILS); - /* Send error only if the first segment arrived. */ - if (fq-last_inFIRST_IN fq-fragments) { - struct net_device *dev = dev_get_by_index(fq-iif); - - /* - But use as source device on which LAST ARRIVED - segment was received. And do not use fq-dev - pointer directly, device might already disappeared. -*/ - if (dev) { - fq-fragments-dev = dev; - icmpv6_send(fq-fragments, ICMPV6_TIME_EXCEED, ICMPV6_EXC_FRAGTIME, 0, - dev); - dev_put(dev); - } - } + /* Don't send error if the first segment did not arrive. */ + if (!(fq-last_inFIRST_IN) || !fq-fragments) + goto out; + + dev = dev_get_by_index(fq-iif); + if (!dev) + goto out; + + /* + But use as source device on which LAST ARRIVED + segment was received. And do not use fq-dev + pointer directly, device might already disappeared. +*/ + fq-fragments-dev = dev; + icmpv6_send(fq-fragments, ICMPV6_TIME_EXCEED, ICMPV6_EXC_FRAGTIME, 0, dev); + dev_put(dev); out: spin_unlock(fq-lock); fq_put(fq, NULL); @@ -368,8 +369,6 @@ ip6_frag_create(unsigned int hash, u32 i if ((fq = frag_alloc_queue()) == NULL) goto oom; - memset(fq, 0, sizeof(struct frag_queue)); - fq-id = id; ipv6_addr_copy(fq-saddr, src); ipv6_addr_copy(fq-daddr, dst); - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] IPv6: Cleanup of net/ipv6/reassambly.c
Hi, On Sunday, 12. March 2006 00:49, Ingo Oeser wrote: From: Ingo Oeser [EMAIL PROTECTED] Two minor cleanups: 1. Using kzalloc() in fraq_alloc_queue() saves the memset() in ipv6_frag_create(). 2. Invert sense of if-statements to streamline code. Inverts the comment, too. These are against net-2.6.17 of course. I also compile tested this and my other kzalloc() changes. Forgot to mention this yesterday... Regards Ingo Oeser - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] IPv6: Cleanups for net/ipv6/addrconf.c (kzalloc, early exit) v2
From: Ingo Oeser [EMAIL PROTECTED] Here are some possible (and trivial) cleanups. - use kzalloc() where possible - invert allocation failure test like if (object) { /* Rest of function here */ } to if (object == NULL) return NULL; /* Rest of function here */ Signed-off-by: Ingo Oeser [EMAIL PROTECTED] Acked-by: YOSHIFUJI Hideaki [EMAIL PROTECTED] --- Hello Dave, On Friday, 10. March 2006 12:02, David S. Miller wrote: This patch no longer applied cleanly, Ingo can you generate a fresh version of your patch against my net-2.6.16 tree? Done! I rebased it, but against net-2.6.17, since it did apply cleanly to net-2.6 and I didn't find the tree for net-2.6.16 as requested. Hope I got it right :-) Regards Ingo Oeser diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c index 8df9eb9..395b3f8 100644 --- a/net/ipv6/addrconf.c +++ b/net/ipv6/addrconf.c @@ -341,84 +341,83 @@ static struct inet6_dev * ipv6_add_dev(s if (dev-mtu IPV6_MIN_MTU) return NULL; - ndev = kmalloc(sizeof(struct inet6_dev), GFP_KERNEL); - - if (ndev) { - memset(ndev, 0, sizeof(struct inet6_dev)); - - rwlock_init(ndev-lock); - ndev-dev = dev; - memcpy(ndev-cnf, ipv6_devconf_dflt, sizeof(ndev-cnf)); - ndev-cnf.mtu6 = dev-mtu; - ndev-cnf.sysctl = NULL; - ndev-nd_parms = neigh_parms_alloc(dev, nd_tbl); - if (ndev-nd_parms == NULL) { - kfree(ndev); - return NULL; - } - /* We refer to the device */ - dev_hold(dev); + ndev = kzalloc(sizeof(struct inet6_dev), GFP_KERNEL); + + if (ndev == NULL) + return NULL; + + rwlock_init(ndev-lock); + ndev-dev = dev; + memcpy(ndev-cnf, ipv6_devconf_dflt, sizeof(ndev-cnf)); + ndev-cnf.mtu6 = dev-mtu; + ndev-cnf.sysctl = NULL; + ndev-nd_parms = neigh_parms_alloc(dev, nd_tbl); + if (ndev-nd_parms == NULL) { + kfree(ndev); + return NULL; + } + /* We refer to the device */ + dev_hold(dev); - if (snmp6_alloc_dev(ndev) 0) { - ADBG((KERN_WARNING - %s(): cannot allocate memory for statistics; dev=%s.\n, - __FUNCTION__, dev-name)); - neigh_parms_release(nd_tbl, ndev-nd_parms); - ndev-dead = 1; - in6_dev_finish_destroy(ndev); - return NULL; - } + if (snmp6_alloc_dev(ndev) 0) { + ADBG((KERN_WARNING + %s(): cannot allocate memory for statistics; dev=%s.\n, + __FUNCTION__, dev-name)); + neigh_parms_release(nd_tbl, ndev-nd_parms); + ndev-dead = 1; + in6_dev_finish_destroy(ndev); + return NULL; + } - if (snmp6_register_dev(ndev) 0) { - ADBG((KERN_WARNING - %s(): cannot create /proc/net/dev_snmp6/%s\n, - __FUNCTION__, dev-name)); - neigh_parms_release(nd_tbl, ndev-nd_parms); - ndev-dead = 1; - in6_dev_finish_destroy(ndev); - return NULL; - } + if (snmp6_register_dev(ndev) 0) { + ADBG((KERN_WARNING + %s(): cannot create /proc/net/dev_snmp6/%s\n, + __FUNCTION__, dev-name)); + neigh_parms_release(nd_tbl, ndev-nd_parms); + ndev-dead = 1; + in6_dev_finish_destroy(ndev); + return NULL; + } - /* One reference from device. We must do this before -* we invoke __ipv6_regen_rndid(). -*/ - in6_dev_hold(ndev); + /* One reference from device. We must do this before +* we invoke __ipv6_regen_rndid(). +*/ + in6_dev_hold(ndev); #ifdef CONFIG_IPV6_PRIVACY - init_timer(ndev-regen_timer); - ndev-regen_timer.function = ipv6_regen_rndid; - ndev-regen_timer.data = (unsigned long) ndev; - if ((dev-flagsIFF_LOOPBACK) || - dev-type == ARPHRD_TUNNEL || - dev-type == ARPHRD_NONE || - dev-type == ARPHRD_SIT) { - printk(KERN_INFO - %s: Disabled Privacy Extensions\n, - dev-name); - ndev-cnf.use_tempaddr = -1; - } else { - in6_dev_hold(ndev); - ipv6_regen_rndid((unsigned long) ndev); - } + init_timer(ndev-regen_timer); + ndev-regen_timer.function
Re: [EXPERIMENTAL] HT aware loopback device (hack, x86-64 only atm)
Andi Kleen wrote: What happens if there are multiple producers? Could happen when this would be used for the socket queue. Then just serialize the producers against each other or try to decouple more. In reality every communication can be modelled with a simple unidirectional pipe or set of pipes. The rest is just historic overdesign (trying to reduce memory consumption, more than one queue is useless-paradigm etc.) biting us now. And a pipe with single producer and a single consumer can be modelled with lock less fifos or net channels, as Van Jacobson calls this. Regards Ingo Oeser - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/8] Intel I/O Acceleration Technology (I/OAT)
Evgeniy Polyakov wrote: On Mon, Mar 06, 2006 at 06:44:07PM +0100, Ingo Oeser ([EMAIL PROTECTED]) wrote: Hmm, so I should resurrect my user page table walker abstraction? There I would hand each page to a recording function, which can drop the page from the collection or coalesce it in the collector if your scatter gather implementation allows it. It depends on where performance growth is stopped. From the first glance it does not look like find_extend_vma(), probably follow_page() fault and thus __handle_mm_fault(). I can not say actually, but if it is true and performance growth is stopped due to increased number of faults and it's processing, your approach will hit this problem too, doesn't it? My approach reduced the number of loops performed and number of memory needed at the expense of doing more work in the main loop of get_user_pages. This was mitigated for the common case of getting just one page by providing a get_one_user_page() function. The whole problem, why we need such multiple loops is that we have no common container object for IO vector + additional data. So we always do a loop working over the vector returned by get_user_pages() all the time. The bigger that vector, the bigger the impact. Maybe sth. as simple as providing get_user_pages() with some offset_of and container_of hackery will work these days without the disadvantages my old get_user_pages() work had. The idea is, that you'll provide a vector (like arguments to calloc) and two offsets: One for the page to store within the offset and one for the vma to store. If the offset has a special value (e.g MAX_LONG) you don't store there at all. But if the performance problem really is get_user_pages() itself (and not its callers), then my approach won't help at all. Regards Ingo Oeser - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: IPv6: avoid dereference of null route entry in ipv6_del_addr()
Hi, YOSHIFUJI Hideaki wrote: In article [EMAIL PROTECTED] (at Mon, 06 Mar 2006 21:50:33 +0100), Jean-Mickael Guerin [EMAIL PROTECTED] says: This patch fixes potential null pointer dereference (I never experiment such crash). The patch is made for net-2.6.17. I disagree. It never happen, because (void *)rt-u.dst is equal to (void *)rt, and dst_release() checks its argument. Since I see nothing, that guarantees that struct rtable will not be reorganized to get better cache access patterns or similiar, I would not trust this very much. What about sth. like this simple defensive patch instead (against Linux 2.6.16-rc4)? Regards Ingo Oeser --- net/ipv6/addrconf.c~2006-02-17 23:23:45.0 +0100 +++ net/ipv6/addrconf.c 2006-03-07 11:19:50.0 +0100 @@ -713,7 +713,8 @@ rt-rt6i_flags |= RTF_EXPIRES; } } - dst_release(rt-u.dst); + if (rt) + dst_release(rt-u.dst); } in6_ifa_put(ifp); - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: IPv6: avoid dereference of null route entry in ipv6_del_addr()
YOSHIFUJI Hideaki wrote: In article [EMAIL PROTECTED] (at Tue, 7 Mar 2006 11:26:13 +0100), Ingo Oeser [EMAIL PROTECTED] says: What about sth. like this simple defensive patch instead (against Linux 2.6.16-rc4)? I disagree again. Sorry. Fine with me. If somebody changes the struct rtable, he'll get a nice Oops while testing ipv6 and the problem won't last long. So now I fully understand, why you keep rejecting this change :-) Thanks for your patience with us. Maybe a comment would be helpful, since this is obviously not obvious. Would you mind queueing a patch nearly citing your first comment like this? --- net/ipv6/addrconf.c~2006-02-17 23:23:45.0 +0100 +++ net/ipv6/addrconf.c 2006-03-07 12:54:41.0 +0100 @@ -713,6 +713,13 @@ rt-rt6i_flags |= RTF_EXPIRES; } } +/* + * We don't mind rt being NULL, + * because (void *)rt-u.dst is equal to (void *)rt, + * and dst_release() checks its argument. + * + * If this assumption changes, we'll notice that quickly. + */ dst_release(rt-u.dst); } Regards Ingo Oeser - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 5/6] x25: Allow 32 bit socket ioctl in 64 bit kernel
Shaun Pereira wrote: removed magic number 33 as suggested by Arnaldo But are you sure, you use the right substitute for it? struct x25_calluserdata { diff -uprN -X dontdiff linux-2.6.16-rc3-vanilla/include/net/x25.h linux-2.6.16-rc3/include/net/x25.h --- linux-2.6.16-rc3-vanilla/include/net/x25.h2006-02-16 15:26:19.0 +1100 +++ linux-2.6.16-rc3/include/net/x25.h2006-02-16 15:31:50.0 +1100 @@ -101,9 +101,17 @@ enum { #define X25_FAC_PACKET_SIZE 0x42 #define X25_FAC_WINDOW_SIZE 0x43 -#define X25_MAX_FAC_LEN 20 /* Plenty to spare */ +#define X25_MAX_FAC_LEN 60 #define X25_MAX_CUD_LEN 128 +#define X25_FAC_CALLING_AE 0xCB +#define X25_FAC_CALLED_AE0xC9 + +#define X25_MARKER 0x00 +#define X25_DTE_SERVICES 0x0F +#define X25_MAX_AE_LEN 40 /* Max num of semi-octets in AE - OSI Nw */ +#define X25_MAX_DTE_FACIL_LEN21 /* Max length of DTE facility params */ Are you sure that you don't mean 0x21 (== 33) here? diff -uprN -X dontdiff linux-2.6.16-rc3-vanilla/net/x25/x25_facilities.c linux-2.6.16-rc3/net/x25/x25_facilities.c --- linux-2.6.16-rc3-vanilla/net/x25/x25_facilities.c 2006-02-16 15:26:25.0 +1100 +++ linux-2.6.16-rc3/net/x25/x25_facilities.c 2006-02-16 15:31:50.0 +1100 @@ -112,9 +127,30 @@ int x25_parse_facilities(struct sk_buff len -= 4; break; case X25_FAC_CLASS_D: - printk(KERN_DEBUG X.25: unknown facility %02X, -length %d, values %02X, %02X, %02X, %02X\n, -p[0], p[1], p[2], p[3], p[4], p[5]); + switch (*p) { + case X25_FAC_CALLING_AE: + if (p[1] X25_MAX_DTE_FACIL_LEN) Because the magic number 33 was here before ... + break; + dte_facs-calling_len = p[2]; + memcpy(dte_facs-calling_ae, p[3], p[1] - 1); + *vc_fac_mask |= X25_MASK_CALLING_AE; + break; + + case X25_FAC_CALLED_AE: + if (p[1] X25_MAX_DTE_FACIL_LEN) ...and here + break; + dte_facs-called_len = p[2]; + memcpy(dte_facs-called_ae, p[3], p[1] - 1); + *vc_fac_mask |= X25_MASK_CALLED_AE; + break; + + default: + printk(KERN_DEBUG X.25: unknown facility %02X, + length %d, values %02X, %02X, %02X, %02X\n, + p[0], p[1], p[2], p[3], p[4], p[5]); + break; + } + len -= p[1] + 2; p += p[1] + 2; break; Regards Ingo Oeser - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] IPv6: Cleanups for net/ipv6/addrconf.c (kzalloc, early exit)
Hello, thanks for your review! On Thursday 09 February 2006 17:48, YOSHIFUJI Hideaki wrote: Did you test your patch? No, just visual inspection, sorry. Please keep nlmsg_failure, which is used by NLMSG_NEW(). Which is a rather nasty macro :-( Ok, will do a new version without that change, which I will run through make allyesconfig make which may take a while. Regards Ingo Oeser - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] IPv6: Cleanups for net/ipv6/addrconf.c (kzalloc, early exit)
From: Ingo Oeser [EMAIL PROTECTED] Here are some possible (and trivial) cleanups. - use kzalloc() where possible - remove unused label - invert allocation failure test like if (object) { /* Rest of function here */ } to if (object == NULL) return NULL; /* Rest of function here */ The last one moves quite some code, because it changes indention. I can split that up, if needed. Signed-off-by: Ingo Oeser [EMAIL PROTECTED] --- diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c index b7d8822..af695f1 100644 --- a/net/ipv6/addrconf.c +++ b/net/ipv6/addrconf.c @@ -327,86 +327,85 @@ static struct inet6_dev * ipv6_add_dev(s if (dev-mtu IPV6_MIN_MTU) return NULL; - ndev = kmalloc(sizeof(struct inet6_dev), GFP_KERNEL); + ndev = kzalloc(sizeof(struct inet6_dev), GFP_KERNEL); - if (ndev) { - memset(ndev, 0, sizeof(struct inet6_dev)); + if (ndev == NULL) + return NULL; - rwlock_init(ndev-lock); - ndev-dev = dev; - memcpy(ndev-cnf, ipv6_devconf_dflt, sizeof(ndev-cnf)); - ndev-cnf.mtu6 = dev-mtu; - ndev-cnf.sysctl = NULL; - ndev-nd_parms = neigh_parms_alloc(dev, nd_tbl); - if (ndev-nd_parms == NULL) { - kfree(ndev); - return NULL; - } - /* We refer to the device */ - dev_hold(dev); + rwlock_init(ndev-lock); + ndev-dev = dev; + memcpy(ndev-cnf, ipv6_devconf_dflt, sizeof(ndev-cnf)); + ndev-cnf.mtu6 = dev-mtu; + ndev-cnf.sysctl = NULL; + ndev-nd_parms = neigh_parms_alloc(dev, nd_tbl); + if (ndev-nd_parms == NULL) { + kfree(ndev); + return NULL; + } + /* We refer to the device */ + dev_hold(dev); - if (snmp6_alloc_dev(ndev) 0) { - ADBG((KERN_WARNING - %s(): cannot allocate memory for statistics; dev=%s.\n, - __FUNCTION__, dev-name)); - neigh_parms_release(nd_tbl, ndev-nd_parms); - ndev-dead = 1; - in6_dev_finish_destroy(ndev); - return NULL; - } + if (snmp6_alloc_dev(ndev) 0) { + ADBG((KERN_WARNING + %s(): cannot allocate memory for statistics; dev=%s.\n, + __FUNCTION__, dev-name)); + neigh_parms_release(nd_tbl, ndev-nd_parms); + ndev-dead = 1; + in6_dev_finish_destroy(ndev); + return NULL; + } - if (snmp6_register_dev(ndev) 0) { - ADBG((KERN_WARNING - %s(): cannot create /proc/net/dev_snmp6/%s\n, - __FUNCTION__, dev-name)); - neigh_parms_release(nd_tbl, ndev-nd_parms); - ndev-dead = 1; - in6_dev_finish_destroy(ndev); - return NULL; - } + if (snmp6_register_dev(ndev) 0) { + ADBG((KERN_WARNING + %s(): cannot create /proc/net/dev_snmp6/%s\n, + __FUNCTION__, dev-name)); + neigh_parms_release(nd_tbl, ndev-nd_parms); + ndev-dead = 1; + in6_dev_finish_destroy(ndev); + return NULL; + } - /* One reference from device. We must do this before -* we invoke __ipv6_regen_rndid(). -*/ - in6_dev_hold(ndev); + /* One reference from device. We must do this before +* we invoke __ipv6_regen_rndid(). +*/ + in6_dev_hold(ndev); #ifdef CONFIG_IPV6_PRIVACY - get_random_bytes(ndev-rndid, sizeof(ndev-rndid)); - get_random_bytes(ndev-entropy, sizeof(ndev-entropy)); - init_timer(ndev-regen_timer); - ndev-regen_timer.function = ipv6_regen_rndid; - ndev-regen_timer.data = (unsigned long) ndev; - if ((dev-flagsIFF_LOOPBACK) || - dev-type == ARPHRD_TUNNEL || - dev-type == ARPHRD_NONE || - dev-type == ARPHRD_SIT) { - printk(KERN_INFO - %s: Disabled Privacy Extensions\n, - dev-name); - ndev-cnf.use_tempaddr = -1; - } else { - in6_dev_hold(ndev); - ipv6_regen_rndid((unsigned long) ndev); - } + get_random_bytes(ndev-rndid, sizeof(ndev-rndid)); + get_random_bytes(ndev-entropy, sizeof(ndev-entropy)); + init_timer(ndev-regen_timer); + ndev-regen_timer.function = ipv6_regen_rndid; + ndev-regen_timer.data
Re: [Bug 5672] New: cannot get socket once accept(2) has failed due to EMFILE/ENFILE
David S. Miller wrote: I haven't found a way to solve the -EFAULT problem, but we should fix the ENFILE/EMFILE issue since we can, this has sat on the backburner long enough :-) This seems to be enough. Is there any valid reason for an application to expect not loosing any connection after EFAULT (which is usally followed by a SEGV)? So I wouldn't worry that much. Regards Ingo Oeser - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH RESEND 3/6] e1000: Added functions to save and restore config
Jesse Brandeburg wrote: On Mon, 23 Jan 2006, Ingo Oeser wrote: Jeff Kirsher wrote: These functions help restore the driver to active configuration when coming out of resume for power management. Shouldn't this problem be fixed in the PCI layer of Linux? PS: CC'ed maintainer of Linux PCI core I recently saw some patches for pci express go by that may actually fix this issue, but until then, this fix in the driver gets us most of the way there. When the subsystem is fixed, we can decide if this code is superfluous and remove it. Agreed. I just wanted to point out the need for such functions to the PCI maintainer and either get a Use function foo() for this!, I have feature $BAR in my queue for this. or Not yet implemented!. Finally I have no objections to fixing the issue in the driver until the PCI layer of Linux offers this functionality itself. Regards Ingo Oeser - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH RESEND 3/6] e1000: Added functions to save and restore config
Jeff Kirsher wrote: These functions help restore the driver to active configuration when coming out of resume for power management. Shouldn't this problem be fixed in the PCI layer of Linux? PS: CC'ed maintainer of Linux PCI core Regards Ingo Oeser - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH RESEND 6/6] e1000: Added driver comments
Jeff Kirsher wrote: Signed-off-by: Jeff Kirsher [EMAIL PROTECTED] Signed-off-by: Jesse Brandeburg [EMAIL PROTECTED] Signed-off-by: John Ronciak [EMAIL PROTECTED] --- drivers/net/e1000/e1000_main.c | 72 +--- 1 files changed, 67 insertions(+), 5 deletions(-) diff --git a/drivers/net/e1000/e1000_main.c b/drivers/net/e1000/e1000_main.c index 44149f9..98a775b 100644 --- a/drivers/net/e1000/e1000_main.c +++ b/drivers/net/e1000/e1000_main.c @@ -29,11 +29,73 @@ #include e1000.h /* Change Log Wasn't it consensus, that changelogs are better kept in version control systems or at least seperate from code? Regards Ingo Oeser - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] signed long - long
Kris Katterjohn wrote: I learned C with KR (ANSI) and it says: C99 seems to be the standard used in kernel. It clarifies much and aligns with real machines in many cases. Regards Ingo Oeser - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC]: IPsec tunnel wildcard addresses
Patrick McHardy wrote: When moving around with my notebook I got annoyed by having to change the IPsec policies whenever I get a new address. This patch handles a tunnel source of 0.0.0.0 as special case and using routing to get the real source address for the acquire message. I've tested with racoon and it works fine. Any objections to this? If this is fully equivalent to the racoon patch, I would like to have this patch in kernel 2.6.16. Rationale: Running with a custom kernel is quite normal these days. Running custom userspace tools is a maintainence nightmare. Getting certain distributions to update these kind of tools to get a usable version is close to impossible. Current workaround for PPP is to restart racoon with rewritten /etc/racoon/racoon-tool.conf in Debian. Regards Ingo Oeser - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC]: IPsec tunnel wildcard addresses
Hi Patrick, hi Herbert, To summarize the advantages of the in-kernel variant: - it behaves similiar to an specific tunnel - you fix it there for ALL IKE daemons - It works with future and current racoon variants so it can go away, when other blocking racoon bugs are fixed and current racoon is accepted into mainline distributions - choosing a tunnel for the default route is done by the kernel which means you can change your default route without asking racoon for this, providing you have already a tunnel established for the old and the new default route The last point makes failover much easier. Regards Ingo Oeser - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Bcm43xx-dev] [Fwd: State of the Union: Wireless]
David S. Miller wrote: From: David Lang [EMAIL PROTECTED] Date: Fri, 6 Jan 2006 14:16:17 -0800 (PST) character devices are far easier to script. this really sounds like the type of configuration stuff that sysfs was designed for. can we avoid yet another configuration tool that's required? netlink is being recommended exactly because it can result in only needing one tool for everything Yes, iproute2 rocks! I recently discovered that it can do xfrm stuff and was amazed to see that the developer(s) had a big clue about what we like to see (and what is human readable), if we type ip xfrm state and ip xfrm policy as opposed to setkey -D and setkey -PD. So I can only hope that netlink and iproute will be chosen as a way to represent it to the user, just because of the clueful developers of iproute2. Regards Ingo Oeser - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] hostap: allow flashing firmware
Jouni Malinen schrieb: [...] that scary comment is there on purpose and it is not fully obsolete. It is still possible to get HFA3841 cards into state which require hardware modifications to recover from (i.e., they are as good as dead for most users). Taken into this account, I would prefer that the help text for HOSTAP_FIRMWARE_NVRAM would include a warning about the potential issues of using incorrect firmware images. In most cases, RAM (volatile) download can be used to upgrade the firmware without having to modify the flash contents. This is also what the current Windows drivers are doing. What about doing it in two steps (first RAM then FLASH)? 1. RAM download 2. Verify that the device works as expected and uses the new firmware 3a. Verifcation ok - download firmware to FLASH 3b. Verifcation failed - Tell user, reset device and use old firmware from FLASH Is this possible or too simple to be true? Regards Ingo Oeser - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: IPSEC tunnel: more than two networks?
Michael Tokarev wrote: [..] So the question is: is the setup like this one supposed to work at all in linux? I know there are other less ugly ways to achieve the same effect, eg by using GRE/IPIP tunnels and incapsulating the traffic into IPSEC (this way, we'll have only one transport-mode IPSEC connection and normal interfaces to route traffic to/via), but this is NOT how the whole infrastrtructure in their network is implemented - they - it seems, for whatever reason - [...] use separate tunnels to route each network. Yes, that's how I did it, too. It works perfectly to tunnel each network segment seperately. Simple routing is not enough. Don't forget to mention your tunneled networks in the FORWARD chain, if your ipsec gateway is also your firewall. I implemented the seperate tunnels via racoon and racoon-tool from latest Debian sarge. Connectivity to a Cisco PIX was possible that way. Regards Ingo Oeser - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html