Re: [PATCH net-2.6.25 3/3] Uninline the inet_twsk_put function

2007-12-21 Thread Ingo Oeser
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

2007-12-20 Thread Ingo Oeser
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

2007-11-09 Thread Ingo Oeser
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

2007-11-09 Thread Ingo Oeser
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

2007-11-07 Thread Ingo Oeser
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

2007-11-07 Thread Ingo Oeser
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

2007-11-01 Thread Ingo Oeser
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.

2007-10-23 Thread Ingo Oeser
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

2007-10-05 Thread Ingo Oeser
(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

2007-10-05 Thread Ingo Oeser
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

2007-09-17 Thread Ingo Oeser
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

2007-09-14 Thread Ingo Oeser
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...

2007-07-06 Thread Ingo Oeser
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.)

2007-05-31 Thread Ingo Oeser
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.

2007-05-24 Thread Ingo Oeser
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.

2007-05-23 Thread Ingo Oeser
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()

2007-05-23 Thread Ingo Oeser
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

2007-05-23 Thread Ingo Oeser
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]

2007-04-27 Thread Ingo Oeser
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

2007-04-25 Thread Ingo Oeser
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]

2007-04-25 Thread Ingo Oeser
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

2007-03-27 Thread Ingo Oeser
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

2007-03-27 Thread Ingo Oeser
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?

2007-03-22 Thread Ingo Oeser
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

2007-03-16 Thread Ingo Oeser
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

2007-02-08 Thread Ingo Oeser
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

2007-02-07 Thread Ingo Oeser
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

2007-02-05 Thread Ingo Oeser
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

2007-02-05 Thread Ingo Oeser
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

2007-01-31 Thread Ingo Oeser
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

2007-01-16 Thread Ingo Oeser
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

2007-01-16 Thread Ingo Oeser
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

2007-01-16 Thread Ingo Oeser
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

2007-01-10 Thread Ingo Oeser
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

2007-01-10 Thread Ingo Oeser
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

2007-01-10 Thread Ingo Oeser
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)

2006-12-15 Thread Ingo Oeser
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

2006-11-08 Thread Ingo Oeser
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

2006-11-07 Thread Ingo Oeser
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

2006-10-24 Thread Ingo Oeser
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)

2006-10-18 Thread Ingo Oeser
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

2006-10-10 Thread Ingo Oeser
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

2006-10-04 Thread Ingo Oeser
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

2006-09-15 Thread Ingo Oeser
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

2006-09-14 Thread Ingo Oeser
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

2006-09-13 Thread Ingo Oeser
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

2006-08-03 Thread Ingo Oeser
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

2006-07-06 Thread Ingo Oeser
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.

2006-06-20 Thread Ingo Oeser
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

2006-05-29 Thread Ingo Oeser
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

2006-05-26 Thread Ingo Oeser
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))

2006-05-26 Thread Ingo Oeser
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))

2006-05-26 Thread Ingo Oeser
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)

2006-05-16 Thread Ingo Oeser
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

2006-05-11 Thread Ingo Oeser
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

2006-05-08 Thread Ingo Oeser
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

2006-05-03 Thread Ingo Oeser
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

2006-05-02 Thread Ingo Oeser
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

2006-04-28 Thread Ingo Oeser
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

2006-04-28 Thread Ingo Oeser
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

2006-04-28 Thread Ingo Oeser
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

2006-04-23 Thread Ingo Oeser
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

2006-04-22 Thread Ingo Oeser
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

2006-04-22 Thread Ingo Oeser
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

2006-04-21 Thread Ingo Oeser
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

2006-04-21 Thread Ingo Oeser
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

2006-04-18 Thread Ingo Oeser
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

2006-04-13 Thread Ingo Oeser
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)

2006-04-12 Thread Ingo Oeser
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

2006-04-11 Thread Ingo Oeser
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

2006-04-11 Thread Ingo Oeser
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

2006-04-11 Thread Ingo Oeser
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

2006-04-10 Thread Ingo Oeser
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

2006-04-10 Thread Ingo Oeser
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

2006-04-07 Thread Ingo Oeser
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

2006-03-31 Thread Ingo Oeser
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...

2006-03-31 Thread Ingo Oeser
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...

2006-03-31 Thread Ingo Oeser
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()

2006-03-13 Thread Ingo Oeser
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

2006-03-11 Thread Ingo Oeser
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

2006-03-11 Thread Ingo Oeser
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

2006-03-11 Thread Ingo Oeser
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

2006-03-10 Thread Ingo Oeser
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)

2006-03-09 Thread Ingo Oeser
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)

2006-03-07 Thread Ingo Oeser
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()

2006-03-07 Thread Ingo Oeser
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()

2006-03-07 Thread Ingo Oeser
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

2006-02-16 Thread Ingo Oeser
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)

2006-02-10 Thread Ingo Oeser
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)

2006-02-09 Thread Ingo Oeser
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

2006-02-01 Thread Ingo Oeser
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

2006-01-26 Thread Ingo Oeser
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

2006-01-23 Thread Ingo Oeser
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

2006-01-23 Thread Ingo Oeser
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

2006-01-17 Thread Ingo Oeser
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

2006-01-12 Thread Ingo Oeser
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

2006-01-12 Thread Ingo Oeser
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]

2006-01-09 Thread Ingo Oeser
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

2006-01-05 Thread Ingo Oeser
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?

2005-12-14 Thread Ingo Oeser
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


  1   2   >