Re: ipfrag calculating its hash outside lock
David S. Miller <[EMAIL PROTECTED]> wrote: > From: Zach Brown <[EMAIL PROTECTED]> > Date: Fri, 07 Apr 2006 16:59:01 -0700 > >> b) Just calculate the hashes under the lock, we're already doing lots of >> work there anyway. > > I think this is the best way to go. Then we don't need to think > about it, and frankly I think the "recheck hash rnd after getting > lock" idea would turn out to be more expensive :) Agreed. We should also fix up ip_frag_create/ip_frag_intern by getting rid of the hash argument and recomputing it in ip_frag_intern. Thanks, -- Visit Openswan at http://www.openswan.org/ Email: Herbert Xu ~{PmV>HI~} <[EMAIL PROTECTED]> Home Page: http://gondor.apana.org.au/~herbert/ PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt - 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] softmac: don't send out packets while scanning
Johannes Berg <[EMAIL PROTECTED]> wrote: > > @@ -239,6 +259,7 @@ void ieee80211softmac_scan_finished(stru >if (net) >sm->set_channel(sm->dev, net->channel); >} > + netif_start_queue(sm->ieee->dev); Please use netif_wake_queue as otherwise packets can get stuck for quite a while. Cheers, -- Visit Openswan at http://www.openswan.org/ Email: Herbert Xu ~{PmV>HI~} <[EMAIL PROTECTED]> Home Page: http://gondor.apana.org.au/~herbert/ PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt - 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: Broadcast ARP packets on link local addresses (Version2).
jamal <[EMAIL PROTECTED]> wrote: > > I think that the broadcast ARP is still valid for any address configured > as a link local addresses to be done in the kernel - for those reasons > described in the RFC (which i didnt find very convincing, but other > people felt strongly about and not having them implies not meeting the > requirement). I like the idea of allowing user-space to control what addresses cause broadcasts. However, I'm uncomfortable with overloading existing flags even though they might appear to fit the bill on the face of it. People may be using this for completely different reasons (address selection) and it's not polite to suddenly turn all their ARPs into broadcasts. So how about a new address flag? We still have some vacancies there. Cheers, -- Visit Openswan at http://www.openswan.org/ Email: Herbert Xu ~{PmV>HI~} <[EMAIL PROTECTED]> Home Page: http://gondor.apana.org.au/~herbert/ PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt - 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: Broadcast ARP packets on link local addresses (Version2).
On Fri, 2006-07-04 at 18:11 -0700, David S. Miller wrote: > > There is actually a desire to move this stuff out of the kernel, especially > now that we have klibc and the like which is a more appropriate place for > things like dhcp, rarp, and bootp. > Makes a lot of sense. > So what it comes down to is in which plane the configuration of these > types of addresses explicitly occur. If userspace does this and > intentionally knows that it is configuring that type of address, then > yes the app should set the attributes correctly. > > That seems to be what you are suggesting. Indeed. I think that the broadcast ARP is still valid for any address configured as a link local addresses to be done in the kernel - for those reasons described in the RFC (which i didnt find very convincing, but other people felt strongly about and not having them implies not meeting the requirement). cheers, jamal - 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: Broadcast ARP packets on link local addresses (Version2).
Sorry - i promised not to respond on this thread; but since i responded to Dave, its only fair i respond to you as well. On Fri, 2006-07-04 at 22:45 +0200, Freek Dijkstra wrote: > jamal wrote: > > > I am assuming you have some form of configuration, no? You should only > > configure an IP address to be link local if you really mean it to be > > so. > > Typically, there is no user-configuration. > Not exactly - there are user space daemons in v4. Other OSes put this stuff in the kernel. > However, it will also broadcast other ARP replies, which is unnecessary. > If that breaks anything -- I do not know. I do not think it will break. > However, it clearly deviates from the behaviour defined in existing > standards, and I would strongly recommend only to deviate from existing > RFC standards if there is a very good reason to do so. > If the RFC claims that V4 link local addresses may conflict in the scenario they described (some strange setup), then it should apply to all V4 link local addresses. > By the way, David posed an interesting question -- what *is* the "scope" > used for now? I could not find a clear answer with Google. Perhaps > someone on the list like to elaborate. > I tried to describe it in previous email: If an address is configured as link local, it is used as the src address in communication when you are communicating in a route that is link local (typically within a single hop) - you may then have more than one route and IP address, of different scope. For going global paths you then use global ip addresses as src. Dont know if that made sense. cheers, jamal - 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: Broadcast ARP packets on link local addresses (Version2).
From: jamal <[EMAIL PROTECTED]> Date: Fri, 07 Apr 2006 21:07:13 -0400 > On Fri, 2006-07-04 at 14:26 -0700, David S. Miller wrote: > > > If an RFC specifically describes the behavior of a specific range of > > IP addresses, we should follow that by default. > > > > The control/ownership of these IP addresses happens in user space; > they are the ones which decide what address to use example: > http://zeroconf.sourceforge.net/?selected=zcip > > unless you put them in the kernel (which is not a bad idea given we have > dhcp client already - some of the MS "OSes" do that), seems to me it > should be up to the user space program to make sure it uses the > addresses in that range. There is actually a desire to move this stuff out of the kernel, especially now that we have klibc and the like which is a more appropriate place for things like dhcp, rarp, and bootp. So what it comes down to is in which plane the configuration of these types of addresses explicitly occur. If userspace does this and intentionally knows that it is configuring that type of address, then yes the app should set the attributes correctly. That seems to be what you are suggesting. - 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: Broadcast ARP packets on link local addresses (Version2).
On Fri, 2006-07-04 at 14:26 -0700, David S. Miller wrote: > If an RFC specifically describes the behavior of a specific range of > IP addresses, we should follow that by default. > The control/ownership of these IP addresses happens in user space; they are the ones which decide what address to use example: http://zeroconf.sourceforge.net/?selected=zcip unless you put them in the kernel (which is not a bad idea given we have dhcp client already - some of the MS "OSes" do that), seems to me it should be up to the user space program to make sure it uses the addresses in that range. > The catch is that the user should still be able to override that > behavior if they wish to. All these programs MUST set link local scope otherwise src address selection wont work as well. If they dont, they are buggy. Anyways, your call Dave - this thread is getting tiring ;-> cheers, jamal - 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: ipfrag calculating its hash outside lock
> I think this is the best way to go. Then we don't need to think > about it, and frankly I think the "recheck hash rnd after getting > lock" idea would turn out to be more expensive :) OK :). I'll throw that together.. - z - 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/21] orinoco_pci: use pci_iomap() for resources
Quoting Francois Romieu <[EMAIL PROTECTED]>: > > > Is there a reason why dev->mem_{start/end} should not be removed ? > > > > Is there a reason why it should? Is it going to be obsolete? > > It is slowly obsoleting for a few years (don't laugh...). It is preferred > to store the relevant address in the private part of the (pci) device. > > Moderately recent drivers do not use it at all. However it's fairly common > in the setup code of the (legacy) isa devices. I agree that many drivers don't use it. But it would be nice to have a document describing what is going on. On one hand we are adding new information elements (such as the bus in "ethtool -i"), on the other hand we are removing addresses from the ifconfig output. Who is deciding which information is useful and which is not? How about netdev->irq? Is it going to be obsolete too? Then I can easily remove orinoco_pci_setup_netdev() with very minimal adjustments. -- Regards, Pavel Roskin - 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: ipfrag calculating its hash outside lock
From: Zach Brown <[EMAIL PROTECTED]> Date: Fri, 07 Apr 2006 16:59:01 -0700 > b) Just calculate the hashes under the lock, we're already doing lots of > work there anyway. I think this is the best way to go. Then we don't need to think about it, and frankly I think the "recheck hash rnd after getting lock" idea would turn out to be more expensive :) - 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] PCI Error Recovery: e100 network device driver
On Fri, Apr 07, 2006 at 06:11:34PM -0500, Linas Vepstas wrote: > --- linux-2.6.17-rc1.orig/drivers/net/e100.c > +++ linux-2.6.17-rc1/drivers/net/e100.c > + * @state: The current pci conneection state connection > +static pci_ers_result_t e100_io_error_detected(struct pci_dev *pdev, > pci_channel_state_t state) > +{ > + struct net_device *netdev = pci_get_drvdata(pdev); > + > + /* Similar to calling e100_down(), but avoids adpater I/O. */ adapter > +static pci_ers_result_t e100_io_slot_reset(struct pci_dev *pdev) > +{ > + struct net_device *netdev = pci_get_drvdata(pdev); > + struct nic *nic = netdev_priv(netdev); > + > + if (pci_enable_device(pdev)) { > + printk(KERN_ERR "e100: Cannot re-enable PCI device after > reset.\n"); > + return PCI_ERS_RESULT_DISCONNECT; > + } > + pci_set_master(pdev); > + > + /* Only one device per card can do a reset */ > + if (0 != PCI_FUNC(pdev->devfn)) Wrong order. - 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] Unaligned accesses in the ethernet bridge
On Thu, 6 Apr 2006 22:37:08 +0200 Adrian Bunk <[EMAIL PROTECTED]> wrote: > On Thu, Mar 23, 2006 at 01:06:02PM +1100, Peter Chubb wrote: > > > > I see lots of > > kernel unaligned access to 0xa001009dbb6f, ip=0xa00100811591 > > kernel unaligned access to 0xa001009dbb6b, ip=0xa001008115c1 > > kernel unaligned access to 0xa001009dbb6d, ip=0xa001008115f1 > > messages in my logs on IA64 when using the ethernet bridge with 2.6.16. > > > > > > Appended is a patch to fix them. > > > I see this patch already made it into 2.6.17-rc1. > > It seems to be a candidate for 2.6.16.3, too? > If yes, please submit it to [EMAIL PROTECTED] The code that caused this was new in 2.6.17 - 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
ipfrag calculating its hash outside lock
I noticed that ip_find() calculates the hash bucket for the incoming fragment using ipfrag_hash_rnd outside the ipfrag_lock. So it can race with ipfrag_secret_rebuild() and end up putting a frag in the previous bucket instead of the new bucket that ipfrag_secret_rebuild() has put the previous frags in. The unlock and write lock acquiry in the allocating case could also hit this. I verified this by spraying a box with 16k udp writes and turning the secret interval way down to hz/100. It didn't take long before it hit. It resulted in reasm failures due to frag timeouts as expected. Before worrying about fixing this I thought I'd sample the crowd. a) Who cares? The interval is huge and it'd be a few packets at most. b) Just calculate the hashes under the lock, we're already doing lots of work there anyway. c) Sample the random value when calculating the hash outside the lock and only recalculate the hash inside the lock if it changes. (Maybe with some memory barrier help). d) Go insane redoing the serialization so that we get rid of this and other suboptimal behaviour. (please no :)). I vote for c) for it's compromise of minimal invasiveness and impact to the unracey case. - z - 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: updated [Patch 1/1] AF_UNIX Datagram getpeersec
Catherine Zhang <[EMAIL PROTECTED]> wrote: > > Enclosed please find the updated AF_UNIX patch. > > ... > > --- linux-2.6.17-rc1/include/asm-alpha/socket.h~lsm-secpeer-unix > 2006-04-03 18:19:47.0 -0400 > +++ linux-2.6.17-rc1-cxzhang/include/asm-alpha/socket.h 2006-04-03 > 18:20:46.0 -0400 > @@ -51,6 +51,7 @@ > #define SCM_TIMESTAMPSO_TIMESTAMP > > #define SO_PEERSEC 30 > +#define SO_PASSSEC 31 Everybody else gets 34, so it might make life easier to give alpha 34 as well? > + case SO_PASSSEC: > + v.val = test_bit(SOCK_PASSSEC, &sock->flags) ? 1 : 0; > + break; I think test_bit() is guaranteed to return 0 or 1. But I wouldn't trust it either ;) > @@ -0,0 +1,52 @@ > +/* > + * SELinux services exported to the rest of the kernel. > + * > + * Author: James Morris <[EMAIL PROTECTED]> > + * > + * Copyright (C) 2005 Red Hat, Inc., James Morris <[EMAIL PROTECTED]> > + * > + * This program is free software; you can redistribute it and/or modify > + * it under the terms of the GNU General Public License version 2, > + * as published by the Free Software Foundation. > + */ > +#include > +#include > +#include > +#include > + > +#include "security.h" > + > +extern int ss_initialized; Please always put extern declarations into header files. > +int selinux_available(void) > +{ > + return ss_initialized; > +} > + > +void selinux_sk_ctxid(struct sock *sk, u32 *ctxid) > +{ > + *ctxid = 0; > + security_sk_info(sk, ctxid, NULL); > +} > + > +void selinux_sock_ctxid(struct socket *sock, u32 *ctxid) > +{ > + *ctxid = 0; > + security_sock_info(sock, ctxid, NULL); > +} > + > +int selinux_ctx_to_id(const char *ctx, u32 *ctxid) > +{ > + return security_context_to_sid(ctx, strlen(ctx), ctxid); > +} > + > +int selinux_id_to_ctx(u32 ctxid, char **ctx, u32 *ctxlen) > +{ > + return security_sid_to_context(ctxid, ctx, ctxlen); > +} I'd suggest that all of these should be inline. > +EXPORT_SYMBOL_GPL(selinux_available); > +EXPORT_SYMBOL_GPL(selinux_sk_ctxid); > +EXPORT_SYMBOL_GPL(selinux_sock_ctxid); > +EXPORT_SYMBOL_GPL(selinux_ctx_to_id); > +EXPORT_SYMBOL_GPL(selinux_id_to_ctx); Please put exports at the line immediately after the exported function. (There are arguments each way, but we should do one or the other, not both). > diff -puN security/selinux/hooks.c~lsm-secpeer-unix security/selinux/hooks.c > --- linux-2.6.17-rc1/security/selinux/hooks.c~lsm-secpeer-unix > 2006-04-04 18:36:25.0 -0400 > +++ linux-2.6.17-rc1-cxzhang/security/selinux/hooks.c 2006-04-06 > 14:35:14.0 -0400 > @@ -3212,6 +3212,29 @@ static int selinux_socket_unix_may_send( > return 0; > } > > +/* Retrieve security info. on sock */ > +void security_sock_info(struct socket *sock, u32 *sid, u16 *class) > +{ > + if (sock) { > + struct inode *inode; > + inode = SOCK_INODE(sock); > + if (inode) { > + struct inode_security_struct *isec; > + isec = inode->i_security; > + if (sid) > + *sid = isec->sid; > + if (class) > + *class = isec->sclass; > + } > + } > +} hm. Allowing the user to pass in 1, 2 or 3 NULLs is a bit unfortunate, and can hide bugs. It'd be nice, if poss, to require that all callers pass in valid pointers. > @@ -96,5 +96,10 @@ int security_fs_use(const char *fstype, > int security_genfs_sid(const char *fstype, char *name, u16 sclass, > u32 *sid); > > +struct sock; > +struct socket; > +void security_sk_info(struct sock *sk, u32 *sid, u16 *class); > +void security_sock_info(struct socket *sock, u32 *sid, u16 *class); > + > #endif /* _SELINUX_SECURITY_H_ */ It's best to put these forward declarations right at the top of the header file. Otherwise we can end up with multiple identical declarations (ie: someone later puts a declaration at line 50 which needs `struct sock', finds that a forward decl is needed). > +++ linux-2.6.17-rc1-cxzhang/include/linux/selinux.h 2006-04-07 > 12:35:49.0 -0400 > @@ -0,0 +1,104 @@ > +/* > + * SELinux services exported to the rest of the kernel. > + * > + * Author: James Morris <[EMAIL PROTECTED]> > + * > + * Copyright (C) 2005 Red Hat, Inc., James Morris <[EMAIL PROTECTED]> > + * > + * This program is free software; you can redistribute it and/or modify > + * it under the terms of the GNU General Public License version 2, > + * as published by the Free Software Foundation. > + */ > +#ifndef _LINUX_SELINUX_H > +#define _LINUX_SELINUX_H > + > +#ifdef CONFIG_SECURITY_SELINUX > + > +struct sock; > +struct socket; > + > +/** > + * selinux_available - check if SELinux is available for use. > + * > + * Returns true if configured, enabled, not disabled and policy loaded. > + */ > +int selinux_available(void); It's more conven
Re: [PATCH 17/21] orinoco_pci: use pci_iomap() for resources
Pavel Roskin <[EMAIL PROTECTED]> : > On Fri, 2006-04-07 at 23:36 +0200, Francois Romieu wrote: > > > > @@ -208,8 +205,8 @@ static int orinoco_pci_init_one(struct p > > > priv = netdev_priv(dev); > > > card = priv->card; > > > card->pci_ioaddr = pci_ioaddr; > > > - dev->mem_start = pci_iorange; > > > - dev->mem_end = pci_iorange + pci_iolen - 1; > > > + dev->mem_start = pci_resource_start(pdev, 0); > > > + dev->mem_end = dev->mem_start + pci_resource_len(pdev, 0) - 1; > > > > Is there a reason why dev->mem_{start/end} should not be removed ? > > Is there a reason why it should? Is it going to be obsolete? It is slowly obsoleting for a few years (don't laugh...). It is preferred to store the relevant address in the private part of the (pci) device. Moderately recent drivers do not use it at all. However it's fairly common in the setup code of the (legacy) isa devices. -- Ueimor - 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
updated [Patch 1/1] AF_UNIX Datagram getpeersec
Hi, James, Stephen, Dave and Chris, Enclosed please find the updated AF_UNIX patch. It addressed three major issues in the previous patch. 1. No directly calling of the SELINUX function security_sid_to_context(). The fix is to export this and other similar functions through wrapper functions in selinux/exports.c. Most of this code is copied from James' outstanding patch: http://people.redhat.com/jmorris/selinux/skfilter/kernel/12-skfilter-selinux-exports.patch 2. Potential performance problem due to the call to sk_callback_lock() in security_sk_sid(). The fix is to introduce a new function selinux_sock_ctxid() that retrieves the sid of a socket w/o locking. 3. scm_send() might generate garbage sid, if SELINUX is not enabled. The fix is to have selinux_id_to_ctx() return ENOTSUPP when SELINUX is not enabled, so that no control message will be generated. Again, my sincere apologies for the delayed response. As always, comments are appreciated! thanks, Catherine From: [EMAIL PROTECTED] This patch implements an API whereby an application can determine the label of its peer's Unix datagram sockets via the auxiliary data mechanism of recvmsg. Patch purpose: This patch enables a security-aware application to retrieve the security context of the peer of a Unix datagram socket. The application can then use this security context to determine the security context for processing on behalf of the peer who sent the packet. Patch design and implementation: The design and implementation is very similar to the UDP case for INET sockets. Basically we build upon the existing Unix domain socket API for retrieving user credentials. Linux offers the API for obtaining user credentials via ancillary messages (i.e., out of band/control messages that are bundled together with a normal message). To retrieve the security context, the application first indicates to the kernel such desire by setting the SO_PASSSEC option via getsockopt. Then the application retrieves the security context using the auxiliary data mechanism. An example server application for Unix datagram socket should look like this: toggle = 1; toggle_len = sizeof(toggle); setsockopt(sockfd, SOL_SOCKET, SO_PASSSEC, &toggle, &toggle_len); recvmsg(sockfd, &msg_hdr, 0); if (msg_hdr.msg_controllen > sizeof(struct cmsghdr)) { cmsg_hdr = CMSG_FIRSTHDR(&msg_hdr); if (cmsg_hdr->cmsg_len <= CMSG_LEN(sizeof(scontext)) && cmsg_hdr->cmsg_level == SOL_SOCKET && cmsg_hdr->cmsg_type == SCM_SECURITY) { memcpy(&scontext, CMSG_DATA(cmsg_hdr), sizeof(scontext)); } } sock_setsockopt is enhanced with a new socket option SOCK_PASSSEC to allow a server socket to receive security context of the peer. Testing: We have tested the patch by setting up Unix datagram client and server applications. We verified that the server can retrieve the security context using the auxiliary data mechanism of recvmsg. --- include/asm-alpha/socket.h |1 include/asm-arm/socket.h|1 include/asm-arm26/socket.h |1 include/asm-cris/socket.h |1 include/asm-frv/socket.h|1 include/asm-h8300/socket.h |1 include/asm-i386/socket.h |1 include/asm-ia64/socket.h |1 include/asm-m32r/socket.h |1 include/asm-m68k/socket.h |1 include/asm-mips/socket.h |1 include/asm-parisc/socket.h |1 include/asm-powerpc/socket.h|1 include/asm-s390/socket.h |1 include/asm-sh/socket.h |1 include/asm-sparc/socket.h |1 include/asm-sparc64/socket.h|1 include/asm-v850/socket.h |1 include/asm-x86_64/socket.h |1 include/asm-xtensa/socket.h |1 include/linux/net.h |1 include/linux/selinux.h | 104 include/net/af_unix.h |2 include/net/scm.h | 13 net/core/sock.c | 11 +++ net/unix/af_unix.c |2 security/selinux/Makefile |2 security/selinux/exports.c | 52 ++ security/selinux/hooks.c| 40 + security/selinux/include/security.h |7 ++ security/selinux/ss/services.c |4 - 31 files changed, 240 insertions(+), 18 deletions(-) diff -puN include/asm-i386/socket.h~lsm-secpeer-unix include/asm-i386/socket.h --- linux-2.6.17-rc1/include/asm-i386/socket.h~lsm-secpeer-unix 2006-04-03 18:19:47.0 -0400 +++ linux-2.6.17-rc1-cxzhang/include/asm-i386/socket.h 2006-04-03 18:20:46.0 -0400 @@ -48,5 +48,6 @@ #define SO_ACCEPTCONN 30 #define SO_PEERSEC 31 +#define SO_PASSSEC 34 #endif /* _ASM_SOCKET_H */ diff -puN include/asm-x86_64/socket.h~lsm-secpeer-unix inc
Re: [PATCH] PCI Error Recovery: e100 network device driver
On Thu, Apr 06, 2006 at 03:46:43PM -0700, Greg KH wrote: > On Thu, Apr 06, 2006 at 05:24:00PM -0500, Linas Vepstas wrote: > > + if(pci_enable_device(pdev)) { > > Add a space after "if" and before "(" please. I guess I'm immune to learning from experience. :-/ Here's a new improved patch. --linas [PATCH] PCI Error Recovery: e100 network device driver Various PCI bus errors can be signaled by newer PCI controllers. This patch adds the PCI error recovery callbacks to the intel ethernet e100 device driver. The patch has been tested, and appears to work well. Signed-off-by: Linas Vepstas <[EMAIL PROTECTED]> Acked-by: Jesse Brandeburg <[EMAIL PROTECTED]> drivers/net/e100.c | 75 + 1 files changed, 75 insertions(+) Index: linux-2.6.17-rc1/drivers/net/e100.c === --- linux-2.6.17-rc1.orig/drivers/net/e100.c2006-04-07 16:21:46.0 -0500 +++ linux-2.6.17-rc1/drivers/net/e100.c 2006-04-07 18:10:52.411266545 -0500 @@ -2780,6 +2780,80 @@ static void e100_shutdown(struct pci_dev DPRINTK(PROBE,ERR, "Error enabling wake\n"); } +/* -- PCI Error Recovery infrastructure -- */ +/** + * e100_io_error_detected - called when PCI error is detected. + * @pdev: Pointer to PCI device + * @state: The current pci conneection state + */ +static pci_ers_result_t e100_io_error_detected(struct pci_dev *pdev, pci_channel_state_t state) +{ + struct net_device *netdev = pci_get_drvdata(pdev); + + /* Similar to calling e100_down(), but avoids adpater I/O. */ + netdev->stop(netdev); + + /* Detach; put netif into state similar to hotplug unplug. */ + netif_poll_enable(netdev); + netif_device_detach(netdev); + + /* Request a slot reset. */ + return PCI_ERS_RESULT_NEED_RESET; +} + +/** + * e100_io_slot_reset - called after the pci bus has been reset. + * @pdev: Pointer to PCI device + * + * Restart the card from scratch. + */ +static pci_ers_result_t e100_io_slot_reset(struct pci_dev *pdev) +{ + struct net_device *netdev = pci_get_drvdata(pdev); + struct nic *nic = netdev_priv(netdev); + + if (pci_enable_device(pdev)) { + printk(KERN_ERR "e100: Cannot re-enable PCI device after reset.\n"); + return PCI_ERS_RESULT_DISCONNECT; + } + pci_set_master(pdev); + + /* Only one device per card can do a reset */ + if (0 != PCI_FUNC(pdev->devfn)) + return PCI_ERS_RESULT_RECOVERED; + e100_hw_reset(nic); + e100_phy_init(nic); + + return PCI_ERS_RESULT_RECOVERED; +} + +/** + * e100_io_resume - resume normal operations + * @pdev: Pointer to PCI device + * + * Resume normal operations after an error recovery + * sequence has been completed. + */ +static void e100_io_resume(struct pci_dev *pdev) +{ + struct net_device *netdev = pci_get_drvdata(pdev); + struct nic *nic = netdev_priv(netdev); + + /* ack any pending wake events, disable PME */ + pci_enable_wake(pdev, 0, 0); + + netif_device_attach(netdev); + if (netif_running(netdev)) { + e100_open(netdev); + mod_timer(&nic->watchdog, jiffies); + } +} + +static struct pci_error_handlers e100_err_handler = { + .error_detected = e100_io_error_detected, + .slot_reset = e100_io_slot_reset, + .resume = e100_io_resume, +}; static struct pci_driver e100_driver = { .name = DRV_NAME, @@ -2791,6 +2865,7 @@ static struct pci_driver e100_driver = { .resume = e100_resume, #endif .shutdown = e100_shutdown, + .err_handler = &e100_err_handler, }; static int __init e100_init_module(void) - 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 16/21] orinoco_pci: disable device and free IRQ when suspending
Pavel Roskin <[EMAIL PROTECTED]> : > On Fri, 2006-04-07 at 23:24 +0200, Francois Romieu wrote: > > Pavel Roskin <[EMAIL PROTECTED]> : > > [...] > > > diff --git a/drivers/net/wireless/orinoco_pci.c > > > b/drivers/net/wireless/orinoco_pci.c > > > index 5362c21..e57e92b 100644 > > > --- a/drivers/net/wireless/orinoco_pci.c > > > +++ b/drivers/net/wireless/orinoco_pci.c > > > @@ -304,7 +304,9 @@ static int orinoco_pci_suspend(struct pc > > > > > > orinoco_unlock(priv, &flags); > > > > > > + free_irq(pdev->irq, dev); > > > pci_save_state(pdev); > > > + pci_disable_device(pdev); > > > pci_set_power_state(pdev, PCI_D3hot); > > > > > > return 0; > > > > /me stares at the thread behind http://lkml.org/lkml/2005/7/30/143 > > > > Imho {free/request}_irq during suspend/resume deserves some > > explanation. > > I followed examples from other drivers. Yep, that's what I do too. tg3/sky2/skge do not free_irq() in the suspend path. They disable the device. > The thread in question deals with the patch where pci_disable_device() > precedes free_irq(). Besides, bridges may need special care because > they pass interrupts from other devices. > I also followed the kernel documentation (Documentation/power/pci.txt), > which says that the driver should free the IRQ on suspend. > > If you can suggest an alternative approach, please do so. I don't see > what I can do differently. Disable the device and avoid free_irq/request_irq altogether ? The documentation does not require more: [...] A driver uses this function to actually transition the device into a low power state. This should include disabling I/O, IRQs, and bus-mastering, as well as ^^ physically transitioning the device to a lower power state; it may also include calls to pci_enable_wake(). -> free_irq() looks like the heavyweight option. request_irq() can fail. The reference implementation does not care (who does ?). Imho it hints that the driver writer should not take the documentation _too_ literally when it suggests that "disabling irq == free_irq". -- Ueimor - 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 19/21] orinoco: reduce differences between PCI drivers, create orinoco_pci.h
On Sat, 2006-04-08 at 00:10 +0200, Francois Romieu wrote: > > + spin_lock_irqsave(&priv->lock, flags); > > Interruptions are enabled. No need to save/restore. Agreed. I have found a few other places where it's also true. I'll fix it. Actually, I'm going to change the locking to accommodate some USB devices, so changes will be more widespread. > > + netif_device_attach(dev); > > + > > + priv->hw_unavailable--; > > + > > + if (priv->open && (! priv->hw_unavailable)) { > > + err = __orinoco_up(dev); > > I wonder if it would be enough to issue hermes_set_irqmask() later > in __orinoco_up() to release this irq disabled section. Maybe, but I prefer not to touch this code, since it's about to undergo a much more radical rework. -- Regards, Pavel Roskin - 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/21] orinoco_pci: use pci_iomap() for resources
On Fri, 2006-04-07 at 23:36 +0200, Francois Romieu wrote: > > @@ -208,8 +205,8 @@ static int orinoco_pci_init_one(struct p > > priv = netdev_priv(dev); > > card = priv->card; > > card->pci_ioaddr = pci_ioaddr; > > - dev->mem_start = pci_iorange; > > - dev->mem_end = pci_iorange + pci_iolen - 1; > > + dev->mem_start = pci_resource_start(pdev, 0); > > + dev->mem_end = dev->mem_start + pci_resource_len(pdev, 0) - 1; > > Is there a reason why dev->mem_{start/end} should not be removed ? Is there a reason why it should? Is it going to be obsolete? -- Regards, Pavel Roskin - 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 19/21] orinoco: reduce differences between PCI drivers, create orinoco_pci.h
Pavel Roskin <[EMAIL PROTECTED]> : > index 000..b05a9a5 > --- /dev/null > +++ b/drivers/net/wireless/orinoco_pci.h [...] > +static int orinoco_pci_resume(struct pci_dev *pdev) > +{ > + struct net_device *dev = pci_get_drvdata(pdev); > + struct orinoco_private *priv = netdev_priv(dev); > + unsigned long flags; > + int err; > + > + pci_set_power_state(pdev, 0); > + pci_enable_device(pdev); > + pci_restore_state(pdev); > + > + err = request_irq(pdev->irq, orinoco_interrupt, SA_SHIRQ, > + dev->name, dev); > + if (err) { > + printk(KERN_ERR "%s: cannot re-allocate IRQ on resume\n", > +dev->name); > + pci_disable_device(pdev); > + return -EBUSY; > + } > + > + err = orinoco_reinit_firmware(dev); > + if (err) { > + printk(KERN_ERR "%s: error %d re-initializing firmware " > +"on resume\n", dev->name, err); > + return err; > + } > + > + spin_lock_irqsave(&priv->lock, flags); Interruptions are enabled. No need to save/restore. > + > + netif_device_attach(dev); > + > + priv->hw_unavailable--; > + > + if (priv->open && (! priv->hw_unavailable)) { > + err = __orinoco_up(dev); I wonder if it would be enough to issue hermes_set_irqmask() later in __orinoco_up() to release this irq disabled section. -- Ueimor - 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 16/21] orinoco_pci: disable device and free IRQ when suspending
Hello! On Fri, 2006-04-07 at 23:24 +0200, Francois Romieu wrote: > Pavel Roskin <[EMAIL PROTECTED]> : > [...] > > diff --git a/drivers/net/wireless/orinoco_pci.c > > b/drivers/net/wireless/orinoco_pci.c > > index 5362c21..e57e92b 100644 > > --- a/drivers/net/wireless/orinoco_pci.c > > +++ b/drivers/net/wireless/orinoco_pci.c > > @@ -304,7 +304,9 @@ static int orinoco_pci_suspend(struct pc > > > > orinoco_unlock(priv, &flags); > > > > + free_irq(pdev->irq, dev); > > pci_save_state(pdev); > > + pci_disable_device(pdev); > > pci_set_power_state(pdev, PCI_D3hot); > > > > return 0; > > /me stares at the thread behind http://lkml.org/lkml/2005/7/30/143 > > Imho {free/request}_irq during suspend/resume deserves some > explanation. I followed examples from other drivers. The thread in question deals with the patch where pci_disable_device() precedes free_irq(). Besides, bridges may need special care because they pass interrupts from other devices. I also followed the kernel documentation (Documentation/power/pci.txt), which says that the driver should free the IRQ on suspend. If you can suggest an alternative approach, please do so. I don't see what I can do differently. -- Regards, Pavel Roskin - 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] netfilter: cannot build latest kernel with CONFIG_NETFILTER=y/m on IA64
From: Brian Haley <[EMAIL PROTECTED]> Date: Fri, 07 Apr 2006 15:37:51 -0400 > Can't build latest 2.6 kernel with CONFIG_NETFILTER=y/m on IA64, there's > a missing #include in net/ipv6/netfilter.c > > net/ipv6/netfilter.c: In function `nf_ip6_checksum': > net/ipv6/netfilter.c:92: warning: implicit declaration of function > `csum_ipv6_magic' ... > Signed-off-by: Brian Haley <[EMAIL PROTECTED]> Applied, thanks a lot brian. - 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] softmac: don't send out packets while scanning
Seems we forgot to stop the queue while scanning. Better do that so we don't transmit packets all the time during background scanning. Signed-off-by: Johannes Berg <[EMAIL PROTECTED]> --- a/net/ieee80211/softmac/ieee80211softmac_scan.c +++ b/net/ieee80211/softmac/ieee80211softmac_scan.c @@ -47,6 +47,7 @@ ieee80211softmac_start_scan(struct ieee8 sm->scanning = 1; spin_unlock_irqrestore(&sm->lock, flags); + netif_stop_queue(sm->ieee->dev); ret = sm->start_scan(sm->dev); if (ret) { spin_lock_irqsave(&sm->lock, flags); @@ -239,6 +259,7 @@ void ieee80211softmac_scan_finished(stru if (net) sm->set_channel(sm->dev, net->channel); } + netif_start_queue(sm->ieee->dev); ieee80211softmac_call_events(sm, IEEE80211SOFTMAC_EVENT_SCAN_FINISHED, NULL); } EXPORT_SYMBOL_GPL(ieee80211softmac_scan_finished); signature.asc Description: This is a digitally signed message part
Re: [PATCH 17/21] orinoco_pci: use pci_iomap() for resources
Pavel Roskin <[EMAIL PROTECTED]> : [...] > diff --git a/drivers/net/wireless/orinoco_pci.c > b/drivers/net/wireless/orinoco_pci.c > index e57e92b..75df90f 100644 > --- a/drivers/net/wireless/orinoco_pci.c > +++ b/drivers/net/wireless/orinoco_pci.c > @@ -170,9 +170,7 @@ static int orinoco_pci_init_one(struct p > const struct pci_device_id *ent) > { > int err = 0; > - unsigned long pci_iorange; > - u16 __iomem *pci_ioaddr = NULL; > - unsigned long pci_iolen; > + void __iomem *pci_ioaddr = NULL; > struct orinoco_private *priv = NULL; > struct orinoco_pci_card *card; > struct net_device *dev = NULL; (ok, the useless initializers disappear later in the serie) [...] > @@ -208,8 +205,8 @@ static int orinoco_pci_init_one(struct p > priv = netdev_priv(dev); > card = priv->card; > card->pci_ioaddr = pci_ioaddr; > - dev->mem_start = pci_iorange; > - dev->mem_end = pci_iorange + pci_iolen - 1; > + dev->mem_start = pci_resource_start(pdev, 0); > + dev->mem_end = dev->mem_start + pci_resource_len(pdev, 0) - 1; Is there a reason why dev->mem_{start/end} should not be removed ? -- Ueimor - 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: Broadcast ARP packets on link local addresses (Version2).
From: Freek Dijkstra <[EMAIL PROTECTED]> Date: Fri, 07 Apr 2006 22:45:46 +0200 > Jamal also wrote: > > Should we hard-code checks for RFC 1918 IPV4 addresses in the kernel? > > After all, they are well defined by the IETF. The answer is no. > > You make a valid argument here: that hard-coding IP address is bad (make > it less flexible). I disagree with this argument. If an RFC specifically describes the behavior of a specific range of IP addresses, we should follow that by default. The catch is that the user should still be able to override that behavior if they wish to. For example, if the RFCs say to treat a certain range of IP addresses as link local, we will do that by default unless a specific scope was passed in by the user. To me this is clearly the right thing to do. - 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 16/21] orinoco_pci: disable device and free IRQ when suspending
Pavel Roskin <[EMAIL PROTECTED]> : [...] > diff --git a/drivers/net/wireless/orinoco_pci.c > b/drivers/net/wireless/orinoco_pci.c > index 5362c21..e57e92b 100644 > --- a/drivers/net/wireless/orinoco_pci.c > +++ b/drivers/net/wireless/orinoco_pci.c > @@ -304,7 +304,9 @@ static int orinoco_pci_suspend(struct pc > > orinoco_unlock(priv, &flags); > > + free_irq(pdev->irq, dev); > pci_save_state(pdev); > + pci_disable_device(pdev); > pci_set_power_state(pdev, PCI_D3hot); > > return 0; /me stares at the thread behind http://lkml.org/lkml/2005/7/30/143 Imho {free/request}_irq during suspend/resume deserves some explanation. -- Ueimor - 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
Fw: [Bugme-new] [Bug 6349] New: iptables DNAT returns unknown error 4294967295
Begin forwarded message: Date: Fri, 7 Apr 2006 06:12:07 -0700 From: [EMAIL PROTECTED] To: [EMAIL PROTECTED] Subject: [Bugme-new] [Bug 6349] New: iptables DNAT returns unknown error 4294967295 http://bugzilla.kernel.org/show_bug.cgi?id=6349 Summary: iptables DNAT returns unknown error 4294967295 Kernel Version: 2.6.17-rc1 Status: NEW Severity: blocking Owner: [EMAIL PROTECTED] Submitter: [EMAIL PROTECTED] Most recent kernel where this bug did not occur: 2.6.16 Distribution: Slackware Hardware Environment: /proc/cpuinfo processor : 0 vendor_id : GenuineIntel cpu family : 15 model : 4 model name : Intel(R) Pentium(R) 4 CPU 3.20GHz stepping: 1 cpu MHz : 3192.676 cache size : 1024 KB physical id : 0 siblings: 2 core id : 0 cpu cores : 1 fdiv_bug: no hlt_bug : no f00f_bug: no coma_bug: no fpu : yes fpu_exception : yes cpuid level : 5 wp : yes flags : fpu vme de pse tsc msr pae mce cx8 apic sep mtrr pge mca cmov pat pse36 clflush dts acpi mmx fxsr sse sse2 ss ht tm pbe constant_tsc pni monitor ds_cpl cid xtpr bogomips: 6392.53 processor : 1 vendor_id : GenuineIntel cpu family : 15 model : 4 model name : Intel(R) Pentium(R) 4 CPU 3.20GHz stepping: 1 cpu MHz : 3192.676 cache size : 1024 KB physical id : 0 siblings: 2 core id : 0 cpu cores : 1 fdiv_bug: no hlt_bug : no f00f_bug: no coma_bug: no fpu : yes fpu_exception : yes cpuid level : 5 wp : yes flags : fpu vme de pse tsc msr pae mce cx8 apic sep mtrr pge mca cmov pat pse36 clflush dts acpi mmx fxsr sse sse2 ss ht tm pbe constant_tsc pni monitor ds_cpl cid xtpr bogomips: 6385.03 00:00.0 Host bridge: Intel Corporation 915G/P/GV/GL/PL/910GL Processor to I/O Controller (rev 04) 00:02.0 VGA compatible controller: Intel Corporation 82915G/GV/910GL Express Chipset Family Graphics Controller (rev 04) 00:1d.0 USB Controller: Intel Corporation 82801FB/FBM/FR/FW/FRW (ICH6 Family) USB UHCI #1 (rev 03) 00:1d.1 USB Controller: Intel Corporation 82801FB/FBM/FR/FW/FRW (ICH6 Family) USB UHCI #2 (rev 03) 00:1d.2 USB Controller: Intel Corporation 82801FB/FBM/FR/FW/FRW (ICH6 Family) USB UHCI #3 (rev 03) 00:1d.3 USB Controller: Intel Corporation 82801FB/FBM/FR/FW/FRW (ICH6 Family) USB UHCI #4 (rev 03) 00:1d.7 USB Controller: Intel Corporation 82801FB/FBM/FR/FW/FRW (ICH6 Family) USB2 EHCI Controller (rev 03) 00:1e.0 PCI bridge: Intel Corporation 82801 PCI Bridge (rev d3) 00:1e.2 Multimedia audio controller: Intel Corporation 82801FB/FBM/FR/FW/FRW (ICH6 Family) AC'97 Audio Controller (rev 03) 00:1f.0 ISA bridge: Intel Corporation 82801FB/FR (ICH6/ICH6R) LPC Interface Bridge (rev 03) 00:1f.2 IDE interface: Intel Corporation 82801FB/FW (ICH6/ICH6W) SATA Controller (rev 03) 00:1f.3 SMBus: Intel Corporation 82801FB/FBM/FR/FW/FRW (ICH6 Family) SMBus Controller (rev 03) 0a:0b.0 Ethernet controller: Broadcom Corporation NetXtreme BCM5705_2 Gigabit Ethernet (rev 03) Software Environment: /proc/version: Linux version 2.6.17-rc1 ([EMAIL PROTECTED]) (gcc version 3.3.6) #1 SMP Fri Apr 7 15:07:33 EEST 2006 ver_linux: Gnu C 3.3.6 Gnu make 3.80 binutils 2.15.92.0.2 util-linux 2.12p mount 2.12p module-init-tools 3.1 e2fsprogs 1.38 reiserfsprogs 3.6.19 reiser4progs line xfsprogs 2.6.13 pcmcia-cs 3.2.8 PPP2.4.4b1 Linux C Library2.3.6 Dynamic linker (ldd) 2.3.6 Linux C++ Library 5.0.7 Procps 3.2.5 Net-tools 1.60 Kbd1.12 Sh-utils 5.2.1 udev 072 Modules Loaded 8250_pnp 8250 serial_core Problem Description: When I try to add the rule with target DNAT to the OUTPUT chain I get the error message: [EMAIL PROTECTED]:~]# iptables -t nat -A OUTPUT -p tcp -d 192.168.1.1 --dport 119 -j DNAT --to-destination 192.168.1.10:8119 iptables: Unknown error 4294967295 No rule is added, and the kernel says the message to the syslog: Apr 7 15:20:05 dbagrii kernel: ip_tables: DNAT target: bad hook_mask 8 This error appears with iptables tool version 1.3.3 and 1.3.5 i tried to use. Steps to reproduce: Run the iptables tool with this arguments: $ iptables -t nat -A OUTPUT -p tcp -d 192.168.1.1 --dport 119 -j DNAT --to-destination 192.168.1.10:8119 --- You are receiving this mail because: --- You are on the CC list for the bug, or are watching someone who is. - 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 vlan structure to user space, take 2
David S. Miller wrote: From: David Acker <[EMAIL PROTECTED]> Date: Fri, 07 Apr 2006 14:12:05 -0400 This is a second attempt at a patch for include/linux/if_vlan.h . Its purpose is to allow a user space program to use the vlan_ethhdr structure when directly handling 802.1Q packets. This can be done by using a raw socket like: int s = socket(PF_PACKET, SOCK_RAW, htons(ETH_P_ALL)); This socket should see VLAN packets from the base interface unchanged (i.e. vlan tag still in packet). I think this really belongs in a portable header file in glibc somewhere. That would work for me. My goal is to have a user space program that can handle vlan packets directly. I guess my take on it was that if_vlan.h is similar to if_ether.h . if_ether.h already exports the ethernet header structure and is available from user space. I thought it made sense to export the vlan header structure in the same manner that the ethernet header structure is exported today. If folks here think glibc's headers are a better place for this, that's fine with me. -Ack - 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: Broadcast ARP packets on link local addresses (Version2).
On Fri, 2006-07-04 at 13:31 -0700, David Daney wrote: > jamal wrote: > > > > RFC 3927 defines what addresses are legit link local scope. The kernel > > says what to do with addresses that are link local scopes. It knows this > > if you tell it is a link local address. > > > > The point that I think you are missing is that RFC 3927 defines what > happens only on the 169.254.0.0/16 subnet and more specifically how ARP > packets should be formed and sent on that particular subnet. > Ok, this is getting to be a cyclic discussion now. There is absolutely no definition of _any_ IPv4 addresses that are supposed to be link local other than the 169.254.0.0/16 defined. We have a flag which labels an IP address to have an attribute of link local. If someone configures an IP address such as 10.0.0.1 link local, that should be fine too. Refer to the example i gave you for RFC 1918 and why you cant stop people from sending ip address 10.0.0.1 to the internet; or the corrolary of why it would be stoopid to do so. > For link local addresses outside of the 169.254.0.0/16 network RFC 3927 > does not apply. > and the RFC said that? > Patching the ARP so that it does RFC 3927 ARP broadcasting for link > local addresses outside of 169.254.0.0/16 would be incorrect. > I apologize, I no longer have the patience to go over this discussion over and over and over again and again. So i wont be responding to any more emails. cheers, jamal - 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: Broadcast ARP packets on link local addresses (Version2).
jamal wrote: > I am assuming you have some form of configuration, no? You should only > configure an IP address to be link local if you really mean it to be > so. Typically, there is no user-configuration. The 169.254.0.0./16 and FE80::0/8 IP ranges are used for so-called zero configuration networking, where the idea is that no configuration is necessary, but devices automatically pick a self-assigned address. Beside PC's, you should think about devices without an interface to make configuration changes at all (e.g. printers of HP, Lexmark and Epson, the Roku soundbridge, etc.). Jamal also wrote: > Should we hard-code checks for RFC 1918 IPV4 addresses in the kernel? > After all, they are well defined by the IETF. The answer is no. You make a valid argument here: that hard-coding IP address is bad (make it less flexible). To avoid possible confusion, we are not talking about RFC 1918 IPv4 addresses though (private use, a.k.a. "Unique Local Unicast"), but about RFC 3927 IPv4 (self-assigned, a.k.a. "Link-Local Unicast"). Your argument still holds for either range of course. Scope can be used, and applications can be changed to require to set the scope when configuring the IP link-local address. One may argue if this is a big deal or not -- in lines of code it is not, but it would be Linux-specific, and I personally like to see it done more or less the same on all OSes. This is a matter of taste. More importantly, the "scope" parameter does not seem appropriate for the purpose of deciding to broadcast or unicast ARP replies. The "scope" parameter, as I understand, gives the scope of an IP addresses: interface-local, link-local, or global. (IPv6 also define site-local and interface-local, but they are not used, and site-local is deprecated now). On the other hand, what it important with regard to RFC 3927 is not if the IP address is link-local or not, but if it is self-assigned or not. Or actually, if there is a change of an address conflict or not, since that is the reason for the ARP broadcasting. It seems to me that the scope does not convey that information, but something which is slightly different: the uniqueness of the address (globally unique, or only unique on a specific link). > If one was to set the scope to be link local, would it work just fine? Fair question. I think it would work fine for 169.254.0.0 provided that the applications are altered. However, it will also broadcast other ARP replies, which is unnecessary. If that breaks anything -- I do not know. I do not think it will break. However, it clearly deviates from the behaviour defined in existing standards, and I would strongly recommend only to deviate from existing RFC standards if there is a very good reason to do so. The only argument in favour of deviating from the standard seems to make the kernel more configurable. To me as a networking person that is not a convincing argument. By the way, David posed an interesting question -- what *is* the "scope" used for now? I could not find a clear answer with Google. Perhaps someone on the list like to elaborate. Regards, Freek - 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: Broadcast ARP packets on link local addresses (Version2).
David Daney wrote: Following your logic through, It seems that you are advocating broadcasting *all* ARP packets on *all* link local addresses. That would mean that on a 192.168.* switched Ethernet network with DHCP that twice as many ARP packets would be broadcast. 192.168.* addresses are not considered "link local", they are rather "private" or "site local" addresses. The scope parameter, as far as I can tell, is used to make routing decisions. Overloading it to also implement the RFC 3927 ARP broadcasting requirement would result in generation of unnecessary network traffic in configurations that are probably the majority of Linux deployments. No extra network traffic, but there is some measurable overhead to looking up the scope in the routing table. One problem is having this type of scheme behave properly by default, i.e. in the absence of user specified entries. Having to create an entry for every interface in the system just to get RFC standard behavior is silly. - Mark - 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: Broadcast ARP packets on link local addresses (Version2).
jamal wrote: On Fri, 2006-07-04 at 11:31 -0700, David Daney wrote: jamal wrote: It exists. Just use it. For testing just use the ip utility. But why does it exist? There must be one or more reasons that it exists. There is only one reason it exists: to define an IP address as being link local. This influences what address is used as the src when going on different paths which also have link scope. It must not exist to cause IPv4 ARP to do broadcast as specified in RFC 3927, or we would not be having this conversation. RFC 3927 defines what addresses are legit link local scope. The kernel says what to do with addresses that are link local scopes. It knows this if you tell it is a link local address. The point that I think you are missing is that RFC 3927 defines what happens only on the 169.254.0.0/16 subnet and more specifically how ARP packets should be formed and sent on that particular subnet. For link local addresses outside of the 169.254.0.0/16 network RFC 3927 does not apply. Patching the ARP so that it does RFC 3927 ARP broadcasting for link local addresses outside of 169.254.0.0/16 would be incorrect. Overloading its current semantics, will cause in unnecessary ARP broadcasts in non RFC 3927 cases. Refer to what i said above. cheers, jamal - 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 vlan structure to user space, take 2
From: David Acker <[EMAIL PROTECTED]> Date: Fri, 07 Apr 2006 14:12:05 -0400 > This is a second attempt at a patch for include/linux/if_vlan.h . > Its purpose is to allow a user space program to use the vlan_ethhdr > structure when directly handling 802.1Q packets. This can be done > by using a raw socket like: > int s = socket(PF_PACKET, SOCK_RAW, htons(ETH_P_ALL)); > This socket should see VLAN packets from the base interface unchanged > (i.e. vlan tag still in packet). I think this really belongs in a portable header file in glibc somewhere. - 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: Broadcast ARP packets on link local addresses (Version2).
On Fri, 2006-07-04 at 11:31 -0700, David Daney wrote: > jamal wrote: > > It exists. Just use it. For testing just use the ip utility. > > But why does it exist? There must be one or more reasons that it exists. > There is only one reason it exists: to define an IP address as being link local. This influences what address is used as the src when going on different paths which also have link scope. > It must not exist to cause IPv4 ARP to do broadcast as specified in RFC > 3927, or we would not be having this conversation. > RFC 3927 defines what addresses are legit link local scope. The kernel says what to do with addresses that are link local scopes. It knows this if you tell it is a link local address. > Overloading its current semantics, will cause in unnecessary ARP > broadcasts in non RFC 3927 cases. > Refer to what i said above. cheers, jamal - 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] ipv4: initialize arp_tbl rw lock
From: Heiko Carstens <[EMAIL PROTECTED]> Date: Fri, 7 Apr 2006 10:15:33 +0200 > From: Heiko Carstens <[EMAIL PROTECTED]> > > The qeth driver makes use of the arp_tbl rw lock. CONFIG_DEBUG_SPINLOCK > detects that this lock is not initialized as it is supposed to be. > > Signed-off-by: Heiko Carstens <[EMAIL PROTECTED]> As Stephen Hemminger pointed out this change is bogus, the lock is initialized at run time by the ARP layer. - 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] netfilter: cannot build latest kernel with CONFIG_NETFILTER=y/m on IA64
Can't build latest 2.6 kernel with CONFIG_NETFILTER=y/m on IA64, there's a missing #include in net/ipv6/netfilter.c net/ipv6/netfilter.c: In function `nf_ip6_checksum': net/ipv6/netfilter.c:92: warning: implicit declaration of function `csum_ipv6_magic' LD net/ipv6/ipv6.o LD net/ipv6/built-in.o LD net/built-in.o GEN .version CHK include/linux/compile.h UPD include/linux/compile.h CC init/version.o LD init/built-in.o LD .tmp_vmlinux1 net/built-in.o(.text+0x168812): In function `nf_ip6_checksum': include/net/checksum.h:67: undefined reference to `csum_ipv6_magic' net/built-in.o(.text+0x168892):include/net/checksum.h:67: undefined reference to `csum_ipv6_magic' Patch against today's net-2.6.git tree. -Brian Signed-off-by: Brian Haley <[EMAIL PROTECTED]> diff --git a/net/ipv6/netfilter.c b/net/ipv6/netfilter.c index 3e9ecfa..395a417 100644 --- a/net/ipv6/netfilter.c +++ b/net/ipv6/netfilter.c @@ -7,6 +7,7 @@ #include #include #include +#include int ip6_route_me_harder(struct sk_buff *skb) {
Re: Broadcast ARP packets on link local addresses (Version2).
jamal wrote: broadcasting requirement would result in generation of unnecessary network traffic in configurations that are probably the majority of Linux deployments. If you choose to configure something to be link local, then you should live with the consequences. 169.254.0.0/16 is by definition link local. I think point made by Janos is we should look at the attributes rather than value. The converse is not true. And that is my problem with this idea. 10.0.0.1 is not supposed to be routed outside either. But we dont stop people who are dumb enough to do it. Have your user space set it to be link local and then fix the kernel if it doesnt do the right thing. I could see adding an additional interface attribute that specifies link local, and then testing that. It exists. Just use it. For testing just use the ip utility. But why does it exist? There must be one or more reasons that it exists. It must not exist to cause IPv4 ARP to do broadcast as specified in RFC 3927, or we would not be having this conversation. Overloading its current semantics, will cause in unnecessary ARP broadcasts in non RFC 3927 cases. David Daney - 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] net: expose vlan structure to user space
From Dave Acker <[EMAIL PROTECTED]> The purpose is to allow a user space program to use the vlan_ethhdr structure when directly handling 802.1Q packets. This can be done by using a raw socket like: int s = socket(PF_PACKET, SOCK_RAW, htons(ETH_P_ALL)); This socket should see VLAN packets from the base interface unchanged (i.e. vlan tag still in packet). Currently other user space programs that understand VLANs seem to all create their own definitions for this structure. This patch does NOT expose the defines related to the sizes of VLAN packets and their fields; it only exposes the vlan_ethhdr structure. It was determined that these sizes are not useful since the packet could be as large the MTU of the underlying physical network. Also, you can have nested VLAN headers such that the actual data offset is greater than the size of the vlan_ethhdr structure. -Dave Acker Signed-off-by: David Acker <[EMAIL PROTECTED]> This is a third attempt at a patch for include/linux/if_vlan.h . I am sorry I got the diff wrong on the previous posts; I am a bit new to linux kernel contributions. This time I believe I fixed the patch format to match the linux kernel doc. Thus, it is taken from above the tree instead of deep within. --- linux-2.6.15.6/include/linux/if_vlan.h.orig 2006-04-07 13:53:53.0 -0400 +++ linux-2.6.15.6/include/linux/if_vlan.h 2006-04-07 14:51:18.0 -0400 @@ -13,6 +13,14 @@ #ifndef _LINUX_IF_VLAN_H_ #define _LINUX_IF_VLAN_H_ +struct vlan_ethhdr { + unsigned char h_dest[ETH_ALEN]; /* destination eth addr */ + unsigned char h_source[ETH_ALEN];/* source ether addr */ + __be16 h_vlan_proto; /* Should always be 0x8100 */ + __be16 h_vlan_TCI;/* Encapsulates priority and VLAN ID */ + unsigned short h_vlan_encapsulated_proto; /* packet type ID field (or len) */ +}; + #ifdef __KERNEL__ /* externally defined structs */ @@ -39,14 +47,6 @@ struct hlist_node; #define VLAN_ETH_DATA_LEN 1500/* Max. octets in payload*/ #define VLAN_ETH_FRAME_LEN 1518/* Max. octets in frame sans FCS */ -struct vlan_ethhdr { - unsigned char h_dest[ETH_ALEN]; /* destination eth addr */ - unsigned char h_source[ETH_ALEN];/* source ether addr */ - __be16 h_vlan_proto; /* Should always be 0x8100 */ - __be16 h_vlan_TCI;/* Encapsulates priority and VLAN ID */ - unsigned short h_vlan_encapsulated_proto; /* packet type ID field (or len) */ -}; - #include static inline struct vlan_ethhdr *vlan_eth_hdr(const struct sk_buff *skb) - 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] softmac: return -EAGAIN from getscan while scanning
Hi, Johannes Berg wrote: > Below patch was developed after discussion with Daniel Drake who > mentioned to me that wireless tools expect an EAGAIN return from getscan > so that they can wait for the scan to finish before printing out the > results. And why do you implement it across two files? I would suggest folding it simply into its sole caller: ieee80211softmac_wx_get_scan_results() in net/ieee80211/softmac/ieee80211softmac_wx.c That should reduce kernel text size a bit. Regards Ingo Oeser - To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] expose vlan structure to user space, take 2
This is a second attempt at a patch for include/linux/if_vlan.h . Its purpose is to allow a user space program to use the vlan_ethhdr structure when directly handling 802.1Q packets. This can be done by using a raw socket like: int s = socket(PF_PACKET, SOCK_RAW, htons(ETH_P_ALL)); This socket should see VLAN packets from the base interface unchanged (i.e. vlan tag still in packet). Currently other user space programs that understand VLANs seem to all create their own definitions for this structure. This patch does NOT expose the defines related to the sizes of VLAN packets and their fields; it only exposes the vlan_ethhdr structure. It was determined that these sizes are not useful since the packet could be as large the MTU of the underlying physical network. Also, you can have nested VLAN headers such that the actual data offset is greater than the size of the vlan_ethhdr structure. -Dave Acker Signed-off-by: David Acker <[EMAIL PROTECTED]> -- *** if_vlan.h.orig 2006-03-19 21:11:01.0 -0500 --- if_vlan.h 2006-04-07 13:51:44.0 -0400 *** *** 13,18 --- 13,26 #ifndef _LINUX_IF_VLAN_H_ #define _LINUX_IF_VLAN_H_ + struct vlan_ethhdr { +unsigned char h_dest[ETH_ALEN]; /* destination eth addr */ +unsigned char h_source[ETH_ALEN];/* source ether addr */ +__be16 h_vlan_proto; /* Should always be 0x8100 */ +__be16 h_vlan_TCI;/* Encapsulates priority and VLAN ID */ +unsigned short h_vlan_encapsulated_proto; /* packet type ID field (or len) */ + }; + #ifdef __KERNEL__ /* externally defined structs */ *** *** 39,52 #define VLAN_ETH_DATA_LEN 1500/* Max. octets in payload*/ #define VLAN_ETH_FRAME_LEN1518/* Max. octets in frame sans FCS */ - struct vlan_ethhdr { -unsigned char h_dest[ETH_ALEN]; /* destination eth addr */ -unsigned char h_source[ETH_ALEN];/* source ether addr */ -__be16 h_vlan_proto; /* Should always be 0x8100 */ -__be16 h_vlan_TCI;/* Encapsulates priority and VLAN ID */ -unsigned short h_vlan_encapsulated_proto; /* packet type ID field (or len) */ - }; - #include static inline struct vlan_ethhdr *vlan_eth_hdr(const struct sk_buff *skb) --- 47,52 - 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: Broadcast ARP packets on link local addresses (Version2).
BTW, what is the point of CCing lkml? This is a networking issue. People interested in networking discussions join netdev. I have taken it, please keep it that way. On Fri, 2006-07-04 at 09:18 -0700, David Daney wrote: > jamal wrote: > > On Thu, 2006-06-04 at 09:17 -0700, David Daney wrote: > > > >>Janos Farkas wrote: > > > > > >>>Sorry for chiming in this late in the discussion, but... Shouldn't it > >>>be more correct to not depend on the ip address of the used network, > >>>but to use the "scope" parameter of the given address? > >>> > >> > > > > Excellent point! It was bothering me as well but i couldnt express my > > view eloquently as you did. > > > > > >>RFC 3927 specifies the Ethernet arp broadcast behavior for only > >>169.254.0.0/16. > > > > > > Thats besides the point. You could, for example, use 1.1.1.1/24 in your > > network instead of the 10.x or 192.x; and i have seen people use 10.x > > in what appears to be public networks. We dont have speacial checks for > > RFC 1918 IP addresses for example. > > > > Following your logic through, It seems that you are advocating > broadcasting *all* ARP packets on *all* link local addresses. That is what i am saying, yes, if you choose to define an arbitrary IPV4 address to be link local. In theory you should only define the 169 address. > That > would mean that on a 192.168.* switched Ethernet network with DHCP that > twice as many ARP packets would be broadcast. > I am not sure i followed that logic. If you define an address to be link local, you should expect to be living with that consequence. > I don't think that is a good idea. On networks with DHCP or statically > allocated addresses, it would be a hard sell to tell people that they > have to broadcast *all* ARP traffic even though there are neither > standards that require nor suggest such action. I am assuming you have some form of configuration, no? You should only configure an IP address to be link local if you really mean it to be so. > The scope parameter, as far as I can tell, is used to make routing > decisions. Overloading it to also implement the RFC 3927 ARP I think you should look at the code. IPV4 address configuration is not the same as routing configuration. The concept of scoping exists in v6 as well. > > broadcasting requirement would result in generation of unnecessary > network traffic in configurations that are probably the majority of > Linux deployments. > If you choose to configure something to be link local, then you should live with the consequences. > > 169.254.0.0/16 is by definition link local. I think point made by Janos > > is we should look at the attributes rather than value. > > > > The converse is not true. And that is my problem with this idea. > 10.0.0.1 is not supposed to be routed outside either. But we dont stop people who are dumb enough to do it. > > Have your user space set it to be link local and then fix the kernel if > > it doesnt do the right thing. > > > > I could see adding an additional interface attribute that specifies link > local, and then testing that. It exists. Just use it. For testing just use the ip utility. > But that adds substantially more overhead > as you would have to allocate space for the attribute somewhere, and > have extra code to allow setting/querying of the attribute from user space. > > The requirement of RFC 3927 is very tightly targeted. My patch does > only what is required, no more. It does not change ARP behavior for > commonly deployed configurations. > A variation of your patch which checks if the address is link local attribute set is what is needed (instead of checking for address 169.x.x.x). User space sets 169.x.x.x to be linklocal when it installs it. cheers, jamal - 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: Broadcast ARP packets on link local addresses (Version2).
On Fri, 2006-07-04 at 18:01 +0200, Freek Dijkstra wrote: > jamal wrote: > > Excellent point! It was bothering me as well but i couldnt express my > > view eloquently as you did. > > Explicitly defining scope may indeed be a better approach at first > sight, but there are a few very practical reasons not to use them. > > First, it would not be useful in this purpose. IPv6 link-local addresses > (FE80::0/10 addresses) are "local" in scope, just like the IPv4 > link-local addresses, but ARP replies for IPv6 should not be broadcasted > (since there are no address conflicts due to the larger address space). > I think you meant ndisc which is a different code base than what the patch provided. So yes, no conflicts in V6 - and therefore this is unneeded. But not so in ipv4. > Secondly, this is matter of choice: either the IP address or the "scope" > parameter is authoritative when it comes to specific behaviour of > network devices. It'd say that since the IETF and IANA specifically use > IP addresses, we should follow that guideline. > This does not at all preclude what the IETF or IANA sets. The difference is mechanism vs policy. Should we hard-code checks for RFC 1918 IPV4 addresses in the kernel? After all, they are well defined by the IETF. The answer is no. This follows the same logic: The controller for v4 link local addressing sits in user space. It sets the values in conformance to to the RFC. > > To elaborate on my last point: > The alternative would be to require all application that set an IP > address to also set the scope. Indeed. What applications are you referring to? As far as i know there's a handful of competing apps which are vying to be the "zero config" daemons on Linux. > This is currently not done. Its a 33.333 seconds fix. > Adding three machine instructions is just the easier approach. I am not objecting to "some fix" in the kernel. I am objecting to the patch that was sent. If one was to set the scope to be link local, wouyld it work just fine? A patch may still be needed with those same number of instructions. The only question is: will it check for an ip address value or an ip address attribute? From a mechanism perspective, attribute sounds a lot reasonable. Lets see if it is even needed if you set the right attribute. [Deleted the rest of the arguement since it resolves around the two above] cheers, jamal - 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: Broadcast ARP packets on link local addresses (Version2).
jamal wrote: On Thu, 2006-06-04 at 09:17 -0700, David Daney wrote: Janos Farkas wrote: Sorry for chiming in this late in the discussion, but... Shouldn't it be more correct to not depend on the ip address of the used network, but to use the "scope" parameter of the given address? Excellent point! It was bothering me as well but i couldnt express my view eloquently as you did. RFC 3927 specifies the Ethernet arp broadcast behavior for only 169.254.0.0/16. Thats besides the point. You could, for example, use 1.1.1.1/24 in your network instead of the 10.x or 192.x; and i have seen people use 10.x in what appears to be public networks. We dont have speacial checks for RFC 1918 IP addresses for example. Following your logic through, It seems that you are advocating broadcasting *all* ARP packets on *all* link local addresses. That would mean that on a 192.168.* switched Ethernet network with DHCP that twice as many ARP packets would be broadcast. I don't think that is a good idea. On networks with DHCP or statically allocated addresses, it would be a hard sell to tell people that they have to broadcast *all* ARP traffic even though there are neither standards that require nor suggest such action. The scope parameter, as far as I can tell, is used to make routing decisions. Overloading it to also implement the RFC 3927 ARP broadcasting requirement would result in generation of unnecessary network traffic in configurations that are probably the majority of Linux deployments. 169.254.0.0/16 is by definition link local. I think point made by Janos is we should look at the attributes rather than value. The converse is not true. And that is my problem with this idea. Have your user space set it to be link local and then fix the kernel if it doesnt do the right thing. I could see adding an additional interface attribute that specifies link local, and then testing that. But that adds substantially more overhead as you would have to allocate space for the attribute somewhere, and have extra code to allow setting/querying of the attribute from user space. The requirement of RFC 3927 is very tightly targeted. My patch does only what is required, no more. It does not change ARP behavior for commonly deployed configurations. Presumably you could set the scope parameter to local for addresses outside of that range or even for protocols other than Ethernet. Since broadcasting ARP packets usually adversely effects usable network bandwidth, we should probably only do it where it is absolutely required. The overhead of testing the value required by the RFC is quite low (3 machine instructions on i686 is the size of the entire patch), so using some proxy like the scope parameter would not even be a performance win. Again, that is beside the point. It may be beside your point, I happen to think that it has some relevance. David Daney - 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: Broadcast ARP packets on link local addresses (Version2).
jamal wrote: >>> Sorry for chiming in this late in the discussion, but... Shouldn't it >>> be more correct to not depend on the ip address of the used network, >>> but to use the "scope" parameter of the given address? > > Excellent point! It was bothering me as well but i couldnt express my > view eloquently as you did. Explicitly defining scope may indeed be a better approach at first sight, but there are a few very practical reasons not to use them. First, it would not be useful in this purpose. IPv6 link-local addresses (FE80::0/10 addresses) are "local" in scope, just like the IPv4 link-local addresses, but ARP replies for IPv6 should not be broadcasted (since there are no address conflicts due to the larger address space). Secondly, this is matter of choice: either the IP address or the "scope" parameter is authoritative when it comes to specific behaviour of network devices. It'd say that since the IETF and IANA specifically use IP addresses, we should follow that guideline. To elaborate on my last point: The alternative would be to require all application that set an IP address to also set the scope. This is currently not done. Adding three machine instructions is just the easier approach. Of course, in some cases the "easy" solution is not always best, but in this case I argue it is. There are IP ranges which are threated as special (see RFC 3330 for IPv4 and RFC 4291 for IPv6). In my opinion, this means there is a strict relation from IP address to scope; defining scope inconsistent with the IP range seems like bad practice to me. For example, using 1.1.1.0/24 would break if IANA would decide to assign that range, and using 10.0.0.0/8 in public networks would simple not work. If someone uses private IP ranges, that's regarded as harmful traffic, and machines of the AS112 project will deal with it (they deploy machines, which you can regard as the /dev/null of the Internet). In short, while scope may be a better argument then ranges, I would not deviate from Internet standards, and would advise against using them both (please refer to the complexity arguments as put forward in "Robustness and the Internet" by Walter Willinger and John Doyle, or simply RFC 1958 and RFC 3439). (*) With regards, Freek Dijkstra (*) Note: using this argument, it is easy to argue that special cases like the proposed patch are bad, and I agree. From a scientific point of view, I would much prefer that everyone use IPv6 and abandon IPv4 yesterday. Unfortunately, that's not practical. - 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: Broadcast ARP packets on link local addresses
On Fri, 2006-07-04 at 08:06 -0600, Mark Butler wrote: > In IPv4, how is this information supposed to be communicated? The > possibilities I can see are: > > 1. Assuming link local based on SO_BINDTODEVICE. > 2. Adding a new field to sockaddr_in in a backward compatible manner > 3. Defining a new RTAX_ route attribute > > Is there something I missed? We already have a scope bitmap to define link local in netlink. Look at include/linux/rtnetlink::struct ifaddrmsg::ifa_scope. It should be a matter of setting that when adding the IP address from user space. iproute ip can already do it; so cutnpaste some of that code in your user space program. cheers, jamal - 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] ipv4: initialize arp_tbl rw lock
On Fri, 7 Apr 2006 10:15:33 +0200 Heiko Carstens <[EMAIL PROTECTED]> wrote: > From: Heiko Carstens <[EMAIL PROTECTED]> > > The qeth driver makes use of the arp_tbl rw lock. CONFIG_DEBUG_SPINLOCK > detects that this lock is not initialized as it is supposed to be. > > Signed-off-by: Heiko Carstens <[EMAIL PROTECTED]> > --- This is a initialization order problem then, because: arp_init neigh_table_init rwlock_init does the initialization already. So fix the initialization sequence of the qeth driver or you will have other problems. My impression was the -rt folks wanted all lock initializations t be done at runtime not compile time. - 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: [XFRM] Restore aevent timer
On Sat, 2006-08-04 at 00:14 +1000, Herbert Xu wrote: > This looks good except that I think the flag beats jiffies since the > latter can wrap around. Of course on any sane setup the SA should've > expired by then but it pays to be safe. > Ok, Lets hear Krisztian's view for a tie breaker. Whichever scheme he likes i would test against. Krisztian? cheers, jamal - 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: Broadcast ARP packets on link local addresses
Mark Butler wrote: jamal wrote: On Fri, 2006-07-04 at 17:20 +1000, Anand Kumria wrote: On Fri, 31 Mar 2006 15:26:00 -0800, David Daney wrote: From: David Daney Greetings, When an internet host joins a network where there is no DHCP server, it may auto-allocate an IP address by the method described in RFC 3927. There are several user space daemons available that implement most of the protocol (zcip, busybox, ...). The kernel's APR driver should function in the normal manner except that it is required to broadcast all ARP packets that it originates in the link local address space (169.254.0.0/16). RFC 3927 section 2.5 explains the requirement. The current ARP code is non-compliant because it does not broadcast some ARP packets as required by RFC 3927. I haven't seem anyone comment on this, Theres a lot of comments - check the archives of netdev on the thread as well as a newer thread under "Broadcast ARP packets on link local addresses (Version2)" but it would be useful to see this integrated. IMO not the way it is defined right now in that patch. As suggested in the thread, best way is for the kernel to check if it is link local and do the advert in broadcast instead of unicast. Something else I've noticed while I've been implementing my zeroconf daemon is that the kernel returns link-scoped primary addresses first to 'ifconfig'. Unfortunately quite a lot of user-space programs parse its output and interpret the address it presents as the primary for the specified interface. Not sure i followed. Is that a case of user-space breakage that the kernel team would ordinarily worry about? I think user space setting the attribute of the address to be link local would be a sufficient hint to the kernel to broadcast the arps. In IPv4, how is this information supposed to be communicated? The possibilities I can see are: 1. Assuming link local based on SO_BINDTODEVICE. 2. Adding a new field to sockaddr_in in a backward compatible manner 3. Defining a new RTAX_ route attribute Excuse me, I didn't realize the routing tables already had an (apparently customizable) attribute for address scope. OF course, why anyone would need to use a non-standard IP address range for link local addresses is a separate issue. Isn't a /16 enough? - Mark B. - 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: Broadcast ARP packets on link local addresses
jamal wrote: On Fri, 2006-07-04 at 17:20 +1000, Anand Kumria wrote: On Fri, 31 Mar 2006 15:26:00 -0800, David Daney wrote: From: David Daney Greetings, When an internet host joins a network where there is no DHCP server, it may auto-allocate an IP address by the method described in RFC 3927. There are several user space daemons available that implement most of the protocol (zcip, busybox, ...). The kernel's APR driver should function in the normal manner except that it is required to broadcast all ARP packets that it originates in the link local address space (169.254.0.0/16). RFC 3927 section 2.5 explains the requirement. The current ARP code is non-compliant because it does not broadcast some ARP packets as required by RFC 3927. I haven't seem anyone comment on this, Theres a lot of comments - check the archives of netdev on the thread as well as a newer thread under "Broadcast ARP packets on link local addresses (Version2)" but it would be useful to see this integrated. IMO not the way it is defined right now in that patch. As suggested in the thread, best way is for the kernel to check if it is link local and do the advert in broadcast instead of unicast. Something else I've noticed while I've been implementing my zeroconf daemon is that the kernel returns link-scoped primary addresses first to 'ifconfig'. Unfortunately quite a lot of user-space programs parse its output and interpret the address it presents as the primary for the specified interface. Not sure i followed. Is that a case of user-space breakage that the kernel team would ordinarily worry about? I think user space setting the attribute of the address to be link local would be a sufficient hint to the kernel to broadcast the arps. In IPv4, how is this information supposed to be communicated? The possibilities I can see are: 1. Assuming link local based on SO_BINDTODEVICE. 2. Adding a new field to sockaddr_in in a backward compatible manner 3. Defining a new RTAX_ route attribute Is there something I missed? - Mark B. - 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: [XFRM] Restore aevent timer
Hi Jamal: On Fri, Apr 07, 2006 at 09:15:41AM -0400, jamal wrote: > > Ok, I built on Herbert's suggestion and tried to be a little > clever/accurate. Instead of a flag i introduce a variable that stores > the jiffy point when the timer is killed. If we fall anywhere to the > right or at exact point of the next point expected timer when the first > packet arrives, then we do an immediate update of type > XFRM_REPLAY_TIMEOUT else we keep receiving packets and wait until the > right time. This looks good except that I think the flag beats jiffies since the latter can wrap around. Of course on any sane setup the SA should've expired by then but it pays to be safe. Cheers, -- Visit Openswan at http://www.openswan.org/ Email: Herbert Xu ~{PmV>HI~} <[EMAIL PROTECTED]> Home Page: http://gondor.apana.org.au/~herbert/ PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt - 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: Broadcast ARP packets on link local addresses
On Fri, 2006-07-04 at 17:20 +1000, Anand Kumria wrote: > On Fri, 31 Mar 2006 15:26:00 -0800, David Daney wrote: > > > From: David Daney > > > > Greetings, > > > > When an internet host joins a network where there is no DHCP server, > > it may auto-allocate an IP address by the method described in RFC > > 3927. There are several user space daemons available that implement > > most of the protocol (zcip, busybox, ...). The kernel's APR driver > > should function in the normal manner except that it is required to > > broadcast all ARP packets that it originates in the link local address > > space (169.254.0.0/16). RFC 3927 section 2.5 explains the requirement. > > > > The current ARP code is non-compliant because it does not broadcast > > some ARP packets as required by RFC 3927. > > I haven't seem anyone comment on this, Theres a lot of comments - check the archives of netdev on the thread as well as a newer thread under "Broadcast ARP packets on link local addresses (Version2)" > but it would be useful to see this > integrated. IMO not the way it is defined right now in that patch. As suggested in the thread, best way is for the kernel to check if it is link local and do the advert in broadcast instead of unicast. > > Something else I've noticed while I've been implementing my zeroconf > daemon is that the kernel returns link-scoped primary addresses first to > 'ifconfig'. Unfortunately quite a lot of user-space programs parse its > output and interpret the address it presents as the primary for the > specified interface. > Not sure i followed. > Is that a case of user-space breakage that the kernel team would > ordinarily worry about? I think user space setting the attribute of the address to be link local would be a sufficient hint to the kernel to broadcast the arps. cheers, jamal - 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: Broadcast ARP packets on link local addresses (Version2).
On Thu, 2006-06-04 at 09:17 -0700, David Daney wrote: > Janos Farkas wrote: > > Sorry for chiming in this late in the discussion, but... Shouldn't it > > be more correct to not depend on the ip address of the used network, > > but to use the "scope" parameter of the given address? > > > Excellent point! It was bothering me as well but i couldnt express my view eloquently as you did. > RFC 3927 specifies the Ethernet arp broadcast behavior for only > 169.254.0.0/16. Thats besides the point. You could, for example, use 1.1.1.1/24 in your network instead of the 10.x or 192.x; and i have seen people use 10.x in what appears to be public networks. We dont have speacial checks for RFC 1918 IP addresses for example. 169.254.0.0/16 is by definition link local. I think point made by Janos is we should look at the attributes rather than value. Have your user space set it to be link local and then fix the kernel if it doesnt do the right thing. > Presumably you could set the scope parameter to local > for addresses outside of that range or even for protocols other than > Ethernet. Since broadcasting ARP packets usually adversely effects > usable network bandwidth, we should probably only do it where it is > absolutely required. The overhead of testing the value required by the > RFC is quite low (3 machine instructions on i686 is the size of the > entire patch), so using some proxy like the scope parameter would not > even be a performance win. > Again, that is beside the point. cheers, jamal - 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: [XFRM] Restore aevent timer
On Thu, 2006-06-04 at 22:13 +0200, KOVACS Krisztian wrote: > Hi, > > On Thursday 06 April 2006 17:18, jamal wrote: > > On Fri, 2006-07-04 at 00:30 +1000, Herbert Xu wrote: > > > If so I see what you mean but I think a better solution is to just > > > set a flag when the XFRM_REPLAY_TIMEOUT fires and nothing has > > > changed. Then when you get XFRM_REPLAY_UPDATE you can notify > > > unconditionally if this flag is set. > > > > > > The flag gets cleared in case of a notification. > > > > That does sound reasonable but i need to think about it a little in > > case it misses some scenario. > > Krisztian, see any leaks with that? > > None, it's fine with me. Ok, I built on Herbert's suggestion and tried to be a little clever/accurate. Instead of a flag i introduce a variable that stores the jiffy point when the timer is killed. If we fall anywhere to the right or at exact point of the next point expected timer when the first packet arrives, then we do an immediate update of type XFRM_REPLAY_TIMEOUT else we keep receiving packets and wait until the right time. I havent tested this so there maybe holes, it compiles though. I would like to test it first (probably at end of day) and then submit; so please review and provide comments which i can incorporate before i punt to Dave. cheers, jamal Send aevent immediately if time since last update is >= threshold timer. Fixes a corner case when packet threshold is very high, the timer low and a very low packet rate input which is bursty. Signed-off-by: Jamal Hadi Salim <[EMAIL PROTECTED]> --- include/net/xfrm.h|3 +++ net/xfrm/xfrm_state.c | 26 -- 2 files changed, 23 insertions(+), 6 deletions(-) diff --git a/include/net/xfrm.h b/include/net/xfrm.h index 0d5529c..82fdc2c 100644 --- a/include/net/xfrm.h +++ b/include/net/xfrm.h @@ -140,6 +140,9 @@ struct xfrm_state /* State for replay detection */ struct xfrm_replay_state replay; + /* keeps track of last time event */ + unsigned long last_time; + /* Replay detection state at the time we sent the last notification */ struct xfrm_replay_state preplay; diff --git a/net/xfrm/xfrm_state.c b/net/xfrm/xfrm_state.c index a8e14dc..61bcffa 100644 --- a/net/xfrm/xfrm_state.c +++ b/net/xfrm/xfrm_state.c @@ -448,8 +448,10 @@ static void __xfrm_state_insert(struct x xfrm_state_hold(x); if (x->replay_maxage && - !mod_timer(&x->rtimer, jiffies + x->replay_maxage)) + !mod_timer(&x->rtimer, jiffies + x->replay_maxage)) { xfrm_state_hold(x); + x->last_time = 0; + } wake_up(&km_waitq); } @@ -806,15 +808,21 @@ void xfrm_replay_notify(struct xfrm_stat if (x->replay_maxdiff && (x->replay.seq - x->preplay.seq < x->replay_maxdiff) && (x->replay.oseq - x->preplay.oseq < x->replay_maxdiff)) - return; + if (x->last_time && + time_after_eq(jiffies, x->replay_maxage+x->last_time)) + event = XFRM_REPLAY_TIMEOUT; + else + return; break; case XFRM_REPLAY_TIMEOUT: if ((x->replay.seq == x->preplay.seq) && (x->replay.bitmap == x->preplay.bitmap) && - (x->replay.oseq == x->preplay.oseq)) + (x->replay.oseq == x->preplay.oseq)) { + x->last_time = jiffies; return; + } break; } @@ -825,8 +833,10 @@ void xfrm_replay_notify(struct xfrm_stat km_state_notify(x, &c); if (x->replay_maxage && - !mod_timer(&x->rtimer, jiffies + x->replay_maxage)) + !mod_timer(&x->rtimer, jiffies + x->replay_maxage)) { xfrm_state_hold(x); + x->last_time = 0; + } } EXPORT_SYMBOL(xfrm_replay_notify); @@ -836,8 +846,12 @@ static void xfrm_replay_timer_handler(un spin_lock(&x->lock); - if (xfrm_aevent_is_on() && x->km.state == XFRM_STATE_VALID) - xfrm_replay_notify(x, XFRM_REPLAY_TIMEOUT); + if (x->km.state == XFRM_STATE_VALID) { + if (xfrm_aevent_is_on()) + xfrm_replay_notify(x, XFRM_REPLAY_TIMEOUT); + else + x->last_time = jiffies; + } spin_unlock(&x->lock); }
Re: Qdisc peek operation
On Thu, 2006-06-04 at 18:29 +0200, Patrick McHardy wrote: [..] > There are basically two possibilities how to implement this. The less > intrusive, but IMO more hackish one is to just handle this inside the > qdiscs that require this operation by not requeueing the packet to > the qdisc, but keeping a private reference somewhere. The disadvantage > is that this distorts statistics and estimators, the classful qdisc > would for example have more packets queued than the sum of all its > inner qdiscs. The other possibility is to introduce a ->peek operation > or a flag to ->dequeue and handle it within the reordering qdiscs. > I think we only need to implement it for non-classful (or single-class) > qdiscs, I can't imagine why anyone would add a scheduler as inner qdisc > to a different scheduler, at least with the current ones. > > Any preferences or suggestions? Otherwise I'll go with the second > possibility. > The second scheme looks better. The other way (mod to the second scheme) is to have the dequeue "compensate for the last time" in the qdiscs i.e if you over/undershot the last time because you computed lets say a delay based on a packet which is no longer at the head of the queue then you take that into account as well; note this suggestion implies it is the non-work-conserving schedulers problem (which i think it is). i.e those schedulers need to be fixed with a new assumption that "the packet you got this time may not be the same as the last one you used to compute next-time-to-give-an-skb". cheers, jamal - 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] Let cassini use ioremap_nocache
> "Michael" == Michael Buesch <[EMAIL PROTECTED]> writes: Michael> If you use pci_iomap, you will have to use its counterpart, Michael> pci_iounmap, too. Indeed... Here is the latest patch. M. This patch (against 2.6.17-rc1) converts the cassini driver to the pci_iomap API, so architectures like PARISC stop screaming about illegal usage of ioremap() on non-cacheable regions. Signed-off-by: Marc Zyngier <[EMAIL PROTECTED]> diff --git a/drivers/net/cassini.c b/drivers/net/cassini.c index ac48f75..39f36aa 100644 --- a/drivers/net/cassini.c +++ b/drivers/net/cassini.c @@ -4877,7 +4877,7 @@ static int __devinit cas_init_one(struct const struct pci_device_id *ent) { static int cas_version_printed = 0; - unsigned long casreg_base, casreg_len; + unsigned long casreg_len; struct net_device *dev; struct cas *cp; int i, err, pci_using_dac; @@ -4972,7 +4972,6 @@ static int __devinit cas_init_one(struct pci_using_dac = 0; } - casreg_base = pci_resource_start(pdev, 0); casreg_len = pci_resource_len(pdev, 0); cp = netdev_priv(dev); @@ -5024,7 +5023,7 @@ static int __devinit cas_init_one(struct cp->timer_ticks = 0; /* give us access to cassini registers */ - cp->regs = ioremap(casreg_base, casreg_len); + cp->regs = pci_iomap(pdev, 0, casreg_len); if (cp->regs == 0UL) { printk(KERN_ERR PFX "Cannot map device registers, " "aborting.\n"); @@ -5123,7 +5122,7 @@ err_out_iounmap: cas_shutdown(cp); mutex_unlock(&cp->pm_mutex); - iounmap(cp->regs); + pci_iounmap(pdev, cp->regs); err_out_free_res: @@ -5171,7 +5170,7 @@ static void __devexit cas_remove_one(str #endif pci_free_consistent(pdev, sizeof(struct cas_init_block), cp->init_block, cp->block_dvma); - iounmap(cp->regs); + pci_iounmap(pdev, cp->regs); free_netdev(dev); pci_release_regions(pdev); pci_disable_device(pdev); -- And if you don't know where you're going, any road will take you there... - 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] ipv4: initialize arp_tbl rw lock
From: Heiko Carstens <[EMAIL PROTECTED]> The qeth driver makes use of the arp_tbl rw lock. CONFIG_DEBUG_SPINLOCK detects that this lock is not initialized as it is supposed to be. Signed-off-by: Heiko Carstens <[EMAIL PROTECTED]> --- net/ipv4/arp.c |1 + 1 file changed, 1 insertion(+) diff --git a/net/ipv4/arp.c b/net/ipv4/arp.c index 041dadd..ea54216 100644 --- a/net/ipv4/arp.c +++ b/net/ipv4/arp.c @@ -202,6 +202,7 @@ struct neigh_table arp_tbl = { .gc_thresh1 = 128, .gc_thresh2 = 512, .gc_thresh3 = 1024, + .lock = RW_LOCK_UNLOCKED, }; int arp_mc_map(u32 addr, u8 *haddr, struct net_device *dev, int dir) - 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 14/21] orinoco: fix BAP0 offset error after several days of operation
From: Jiri Benc <[EMAIL PROTECTED]> After several days of operation of Netgear MA311 card, the card becomes to seek improperly and needs reset. This patch tries to reset the card when this situation occurs. Mar 9 06:45:16 berkeley kernel: wlan0: Error -5 writing packet to BAP Mar 9 06:45:16 berkeley kernel: hermes @ f992a000: BAP0 offset error: reg=0x4044 id=0x128 offset=0x44 Mar 9 06:45:16 berkeley kernel: wlan0: Error -5 writing packet to BAP Mar 9 06:45:16 berkeley kernel: hermes @ f992a000: BAP0 offset error: reg=0x4044 id=0x128 offset=0x44 (etc.) A more detailed description of the problem can be found at https://bugzilla.novell.com/show_bug.cgi?id=154773 The same problem with different card is reported at http://sourceforge.net/mailarchive/message.php?msg_id=14597046 Signed-off-by: Jiri Benc <[EMAIL PROTECTED]> Signed-off-by: Pavel Roskin <[EMAIL PROTECTED]> --- drivers/net/wireless/orinoco.c |2 ++ 1 files changed, 2 insertions(+), 0 deletions(-) diff --git a/drivers/net/wireless/orinoco.c b/drivers/net/wireless/orinoco.c index 4d63738..80cf6fb 100644 --- a/drivers/net/wireless/orinoco.c +++ b/drivers/net/wireless/orinoco.c @@ -536,6 +536,8 @@ static int orinoco_xmit(struct sk_buff * return NETDEV_TX_OK; busy: + if (err == -EIO) + schedule_work(&priv->reset_work); orinoco_unlock(priv, &flags); return NETDEV_TX_BUSY; } - 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 07/21] orinoco: Symbol card supported by spectrum_cs is LA4137, not LA4100
From: Pavel Roskin <[EMAIL PROTECTED]> Signed-off-by: Pavel Roskin <[EMAIL PROTECTED]> --- drivers/net/wireless/spectrum_cs.c |4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/net/wireless/spectrum_cs.c b/drivers/net/wireless/spectrum_cs.c index 58c153a..14f4daa 100644 --- a/drivers/net/wireless/spectrum_cs.c +++ b/drivers/net/wireless/spectrum_cs.c @@ -1,6 +1,6 @@ /* * Driver for 802.11b cards using RAM-loadable Symbol firmware, such as - * Symbol Wireless Networker LA4100, CompactFlash cards by Socket + * Symbol Wireless Networker LA4137, CompactFlash cards by Socket * Communications and Intel PRO/Wireless 2011B. * * The driver implements Symbol firmware download. The rest is handled @@ -958,7 +958,7 @@ static char version[] __initdata = DRIVE " David Gibson <[EMAIL PROTECTED]>, et al)"; static struct pcmcia_device_id spectrum_cs_ids[] = { - PCMCIA_DEVICE_MANF_CARD(0x026c, 0x0001), /* Symbol Spectrum24 LA4100 */ + PCMCIA_DEVICE_MANF_CARD(0x026c, 0x0001), /* Symbol Spectrum24 LA4137 */ PCMCIA_DEVICE_MANF_CARD(0x0104, 0x0001), /* Socket Communications CF */ PCMCIA_DEVICE_PROD_ID12("Intel", "PRO/Wireless LAN PC Card", 0x816cc815, 0x6fbf459a), /* 2011B, not 2011 */ PCMCIA_DEVICE_NULL, - 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 08/21] orinoco: optimize Tx exception handling in orinoco
From: Pavel Roskin <[EMAIL PROTECTED]> When processing Tx exception, only read data until addr1. Rename hermes_tx_descriptor_802_11 to hermes_txexc_data since it's only used to Tx exceptions. Reuse existing hermes_tx_descriptor structure. Remove fields after addr1 - they are not read from the card. Signed-off-by: Pavel Roskin <[EMAIL PROTECTED]> --- drivers/net/wireless/orinoco.c | 42 ++-- 1 files changed, 6 insertions(+), 36 deletions(-) diff --git a/drivers/net/wireless/orinoco.c b/drivers/net/wireless/orinoco.c index 9550bf3..ceea494 100644 --- a/drivers/net/wireless/orinoco.c +++ b/drivers/net/wireless/orinoco.c @@ -201,41 +201,12 @@ #define BITRATE_TABLE_SIZE ARRAY_SIZE(bi /* Data types */ // -/* Used in Event handling. - * We avoid nested structures as they break on ARM -- Moustafa */ -struct hermes_tx_descriptor_802_11 { - /* hermes_tx_descriptor */ - __le16 status; - __le16 reserved1; - __le16 reserved2; - __le32 sw_support; - u8 retry_count; - u8 tx_rate; - __le16 tx_control; - - /* ieee80211_hdr */ +/* Beginning of the Tx descriptor, used in TxExc handling */ +struct hermes_txexc_data { + struct hermes_tx_descriptor desc; __le16 frame_ctl; __le16 duration_id; u8 addr1[ETH_ALEN]; - u8 addr2[ETH_ALEN]; - u8 addr3[ETH_ALEN]; - __le16 seq_ctl; - u8 addr4[ETH_ALEN]; - - __le16 data_len; - - /* ethhdr */ - u8 h_dest[ETH_ALEN];/* destination eth addr */ - u8 h_source[ETH_ALEN]; /* source ether addr*/ - __be16 h_proto; /* packet type ID field */ - - /* p8022_hdr */ - u8 dsap; - u8 ssap; - u8 ctrl; - u8 oui[3]; - - __be16 ethertype; } __attribute__ ((packed)); /* Rx frame header except compatibility 802.3 header */ @@ -620,7 +591,7 @@ static void __orinoco_ev_txexc(struct ne struct net_device_stats *stats = &priv->stats; u16 fid = hermes_read_regn(hw, TXCOMPLFID); u16 status; - struct hermes_tx_descriptor_802_11 hdr; + struct hermes_txexc_data hdr; int err = 0; if (fid == DUMMY_FID) @@ -628,8 +599,7 @@ static void __orinoco_ev_txexc(struct ne /* Read part of the frame header - we need status and addr1 */ err = hermes_bap_pread(hw, IRQ_BAP, &hdr, - offsetof(struct hermes_tx_descriptor_802_11, - addr2), + sizeof(struct hermes_txexc_data), fid, 0); hermes_write_regn(hw, TXCOMPLFID, DUMMY_FID); @@ -649,7 +619,7 @@ static void __orinoco_ev_txexc(struct ne * exceeded, because that's the only status that really mean * that this particular node went away. * Other errors means that *we* screwed up. - Jean II */ - status = le16_to_cpu(hdr.status); + status = le16_to_cpu(hdr.desc.status); if (status & (HERMES_TXSTAT_RETRYERR | HERMES_TXSTAT_AGEDERR)) { union iwreq_datawrqu; - 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 13/21] orinoco: simplify 802.3 encapsulation code
From: Pavel Roskin <[EMAIL PROTECTED]> Use skb_pull() to strip the addresses from the original packet. Don't strip protocol bytes. Signed-off-by: Pavel Roskin <[EMAIL PROTECTED]> --- drivers/net/wireless/orinoco.c | 44 ++-- drivers/net/wireless/orinoco.h | 14 - 2 files changed, 20 insertions(+), 38 deletions(-) diff --git a/drivers/net/wireless/orinoco.c b/drivers/net/wireless/orinoco.c index e7d06b9..4d63738 100644 --- a/drivers/net/wireless/orinoco.c +++ b/drivers/net/wireless/orinoco.c @@ -421,9 +421,8 @@ static int orinoco_xmit(struct sk_buff * hermes_t *hw = &priv->hw; int err = 0; u16 txfid = priv->txfid; - char *p; struct ethhdr *eh; - int data_len, data_off; + int data_off; struct hermes_tx_descriptor desc; unsigned long flags; @@ -453,8 +452,7 @@ static int orinoco_xmit(struct sk_buff * } /* Check packet length */ - data_len = skb->len; - if (data_len < ETH_HLEN) + if (skb->len < ETH_HLEN) goto drop; eh = (struct ethhdr *)skb->data; @@ -477,22 +475,22 @@ static int orinoco_xmit(struct sk_buff * /* Encapsulate Ethernet-II frames */ if (ntohs(eh->h_proto) > ETH_DATA_LEN) { /* Ethernet-II frame */ - struct header_struct hdr; - data_len = skb->len - ETH_HLEN; - data_off = HERMES_802_3_OFFSET + sizeof(hdr); - p = skb->data + ETH_HLEN; - - /* 802.3 header */ - memcpy(hdr.dest, eh->h_dest, ETH_ALEN); - memcpy(hdr.src, eh->h_source, ETH_ALEN); - hdr.len = htons(data_len + ENCAPS_OVERHEAD); - - /* 802.2 header */ - memcpy(&hdr.dsap, &encaps_hdr, sizeof(encaps_hdr)); - - hdr.ethertype = eh->h_proto; - err = hermes_bap_pwrite(hw, USER_BAP, &hdr, sizeof(hdr), -txfid, HERMES_802_3_OFFSET); + struct header_struct { + struct ethhdr eth; /* 802.3 header */ + u8 encap[6];/* 802.2 header */ + } __attribute__ ((packed)) hdr; + + /* Strip destination and source from the data */ + skb_pull(skb, 2 * ETH_ALEN); + data_off = HERMES_802_2_OFFSET + sizeof(encaps_hdr); + + /* And move them to a separate header */ + memcpy(&hdr.eth, eh, 2 * ETH_ALEN); + hdr.eth.h_proto = htons(sizeof(encaps_hdr) + skb->len); + memcpy(hdr.encap, encaps_hdr, sizeof(encaps_hdr)); + + err = hermes_bap_pwrite(hw, USER_BAP, &hdr, sizeof(hdr), + txfid, HERMES_802_3_OFFSET); if (err) { if (net_ratelimit()) printk(KERN_ERR "%s: Error %d writing packet " @@ -500,12 +498,10 @@ static int orinoco_xmit(struct sk_buff * goto busy; } } else { /* IEEE 802.3 frame */ - data_len = skb->len; data_off = HERMES_802_3_OFFSET; - p = skb->data; } - err = hermes_bap_pwrite(hw, USER_BAP, p, data_len, + err = hermes_bap_pwrite(hw, USER_BAP, skb->data, skb->len, txfid, data_off); if (err) { printk(KERN_ERR "%s: Error %d writing packet to BAP\n", @@ -527,7 +523,7 @@ static int orinoco_xmit(struct sk_buff * } dev->trans_start = jiffies; - stats->tx_bytes += data_off + data_len; + stats->tx_bytes += data_off + skb->len; goto ok; drop: diff --git a/drivers/net/wireless/orinoco.h b/drivers/net/wireless/orinoco.h index c6922a7..ca01e45 100644 --- a/drivers/net/wireless/orinoco.h +++ b/drivers/net/wireless/orinoco.h @@ -30,20 +30,6 @@ struct orinoco_key { char data[ORINOCO_MAX_KEY_SIZE]; } __attribute__ ((packed)); -struct header_struct { - /* 802.3 */ - u8 dest[ETH_ALEN]; - u8 src[ETH_ALEN]; - __be16 len; - /* 802.2 */ - u8 dsap; - u8 ssap; - u8 ctrl; - /* SNAP */ - u8 oui[3]; - unsigned short ethertype; -} __attribute__ ((packed)); - typedef enum { FIRMWARE_TYPE_AGERE, FIRMWARE_TYPE_INTERSIL, - 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 21/21] orinoco: bump version to 0.15
From: Pavel Roskin <[EMAIL PROTECTED]> Signed-off-by: Pavel Roskin <[EMAIL PROTECTED]> --- drivers/net/wireless/orinoco.h |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/drivers/net/wireless/orinoco.h b/drivers/net/wireless/orinoco.h index ca01e45..16db3e1 100644 --- a/drivers/net/wireless/orinoco.h +++ b/drivers/net/wireless/orinoco.h @@ -7,7 +7,7 @@ #ifndef _ORINOCO_H #define _ORINOCO_H -#define DRIVER_VERSION "0.15rc3" +#define DRIVER_VERSION "0.15" #include #include - 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 12/21] orinoco: refactor and clean up Tx error handling
From: Pavel Roskin <[EMAIL PROTECTED]> The result of orinoco_xmit() can be OK, dropped packet and busy transmitter. Rename labels accordingly. Increment stats->tx_errors in one place. Increment stats->tx_dropped - nobody is doing it for us. Signed-off-by: Pavel Roskin <[EMAIL PROTECTED]> --- drivers/net/wireless/orinoco.c | 29 + 1 files changed, 13 insertions(+), 16 deletions(-) diff --git a/drivers/net/wireless/orinoco.c b/drivers/net/wireless/orinoco.c index 9fde17b..e7d06b9 100644 --- a/drivers/net/wireless/orinoco.c +++ b/drivers/net/wireless/orinoco.c @@ -449,16 +449,13 @@ static int orinoco_xmit(struct sk_buff * /* Oops, the firmware hasn't established a connection, silently drop the packet (this seems to be the safest approach). */ - stats->tx_errors++; - orinoco_unlock(priv, &flags); - dev_kfree_skb(skb); - return NETDEV_TX_OK; + goto drop; } /* Check packet length */ data_len = skb->len; if (data_len < ETH_HLEN) - goto fail; + goto drop; eh = (struct ethhdr *)skb->data; @@ -469,8 +466,7 @@ static int orinoco_xmit(struct sk_buff * if (net_ratelimit()) printk(KERN_ERR "%s: Error %d writing Tx descriptor " "to BAP\n", dev->name, err); - stats->tx_errors++; - goto fail; + goto busy; } /* Clear the 802.11 header and data length fields - some @@ -501,8 +497,7 @@ static int orinoco_xmit(struct sk_buff * if (net_ratelimit()) printk(KERN_ERR "%s: Error %d writing packet " "header to BAP\n", dev->name, err); - stats->tx_errors++; - goto fail; + goto busy; } } else { /* IEEE 802.3 frame */ data_len = skb->len; @@ -515,8 +510,7 @@ static int orinoco_xmit(struct sk_buff * if (err) { printk(KERN_ERR "%s: Error %d writing packet to BAP\n", dev->name, err); - stats->tx_errors++; - goto fail; + goto busy; } /* Finally, we actually initiate the send */ @@ -529,20 +523,23 @@ static int orinoco_xmit(struct sk_buff * if (net_ratelimit()) printk(KERN_ERR "%s: Error %d transmitting packet\n", dev->name, err); - stats->tx_errors++; - goto fail; + goto busy; } dev->trans_start = jiffies; stats->tx_bytes += data_off + data_len; + goto ok; - orinoco_unlock(priv, &flags); + drop: + stats->tx_errors++; + stats->tx_dropped++; + ok: + orinoco_unlock(priv, &flags); dev_kfree_skb(skb); - return NETDEV_TX_OK; - fail: + busy: orinoco_unlock(priv, &flags); return NETDEV_TX_BUSY; } - 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 18/21] orinoco: support PCI suspend/resume for Nortel, PLX and TMD adaptors
From: Pavel Roskin <[EMAIL PROTECTED]> Copy PCI suspend/resume functions from orinoco_pci.c. Signed-off-by: Pavel Roskin <[EMAIL PROTECTED]> --- drivers/net/wireless/orinoco_nortel.c | 87 +++-- drivers/net/wireless/orinoco_plx.c| 79 ++ drivers/net/wireless/orinoco_tmd.c| 79 ++ 3 files changed, 241 insertions(+), 4 deletions(-) diff --git a/drivers/net/wireless/orinoco_nortel.c b/drivers/net/wireless/orinoco_nortel.c index d1a670b..3fff013 100644 --- a/drivers/net/wireless/orinoco_nortel.c +++ b/drivers/net/wireless/orinoco_nortel.c @@ -263,6 +263,83 @@ static void __devexit nortel_pci_remove_ pci_disable_device(pdev); } +static int orinoco_nortel_suspend(struct pci_dev *pdev, pm_message_t state) +{ + struct net_device *dev = pci_get_drvdata(pdev); + struct orinoco_private *priv = netdev_priv(dev); + unsigned long flags; + int err; + + err = orinoco_lock(priv, &flags); + if (err) { + printk(KERN_ERR "%s: cannot lock hardware for suspend\n", + dev->name); + return err; + } + + err = __orinoco_down(dev); + if (err) + printk(KERN_WARNING "%s: error %d bringing interface down " + "for suspend\n", dev->name, err); + + netif_device_detach(dev); + + priv->hw_unavailable++; + + orinoco_unlock(priv, &flags); + + free_irq(pdev->irq, dev); + pci_save_state(pdev); + pci_disable_device(pdev); + pci_set_power_state(pdev, PCI_D3hot); + + return 0; +} + +static int orinoco_nortel_resume(struct pci_dev *pdev) +{ + struct net_device *dev = pci_get_drvdata(pdev); + struct orinoco_private *priv = netdev_priv(dev); + unsigned long flags; + int err; + + pci_set_power_state(pdev, 0); + pci_enable_device(pdev); + pci_restore_state(pdev); + + err = request_irq(pdev->irq, orinoco_interrupt, SA_SHIRQ, + dev->name, dev); + if (err) { + printk(KERN_ERR "%s: cannot re-allocate IRQ on resume\n", + dev->name); + pci_disable_device(pdev); + return -EBUSY; + } + + err = orinoco_reinit_firmware(dev); + if (err) { + printk(KERN_ERR "%s: error %d re-initializing firmware " + "on resume\n", dev->name, err); + return err; + } + + spin_lock_irqsave(&priv->lock, flags); + + netif_device_attach(dev); + + priv->hw_unavailable--; + + if (priv->open && (! priv->hw_unavailable)) { + err = __orinoco_up(dev); + if (err) + printk(KERN_ERR "%s: Error %d restarting card on resume\n", + dev->name, err); + } + + spin_unlock_irqrestore(&priv->lock, flags); + + return 0; +} static struct pci_device_id nortel_pci_id_table[] = { /* Nortel emobility PCI */ @@ -275,10 +352,12 @@ static struct pci_device_id nortel_pci_i MODULE_DEVICE_TABLE(pci, nortel_pci_id_table); static struct pci_driver nortel_pci_driver = { - .name = DRIVER_NAME, - .id_table = nortel_pci_id_table, - .probe = nortel_pci_init_one, - .remove = __devexit_p(nortel_pci_remove_one), + .name = DRIVER_NAME, + .id_table = nortel_pci_id_table, + .probe = nortel_pci_init_one, + .remove = __devexit_p(nortel_pci_remove_one), + .suspend= orinoco_nortel_suspend, + .resume = orinoco_nortel_resume, }; static char version[] __initdata = DRIVER_NAME " " DRIVER_VERSION diff --git a/drivers/net/wireless/orinoco_plx.c b/drivers/net/wireless/orinoco_plx.c index 210e737..3fe7a2f 100644 --- a/drivers/net/wireless/orinoco_plx.c +++ b/drivers/net/wireless/orinoco_plx.c @@ -343,6 +343,83 @@ static void __devexit orinoco_plx_remove pci_disable_device(pdev); } +static int orinoco_plx_suspend(struct pci_dev *pdev, pm_message_t state) +{ + struct net_device *dev = pci_get_drvdata(pdev); + struct orinoco_private *priv = netdev_priv(dev); + unsigned long flags; + int err; + + err = orinoco_lock(priv, &flags); + if (err) { + printk(KERN_ERR "%s: cannot lock hardware for suspend\n", + dev->name); + return err; + } + + err = __orinoco_down(dev); + if (err) + printk(KERN_WARNING "%s: error %d bringing interface down " + "for suspend\n", dev->name, err); + + netif_device_detach(dev); + + priv->hw_unavailable++; + + orinoco_unlock(priv, &flags); + + free_irq(pdev->irq, dev); + pci_save_state(pdev); + pci_disable_device(pdev); + pci_set_power_state(pdev, PCI_D
[PATCH 16/21] orinoco_pci: disable device and free IRQ when suspending
From: Pavel Roskin <[EMAIL PROTECTED]> Signed-off-by: Pavel Roskin <[EMAIL PROTECTED]> --- drivers/net/wireless/orinoco_pci.c | 11 +++ 1 files changed, 11 insertions(+), 0 deletions(-) diff --git a/drivers/net/wireless/orinoco_pci.c b/drivers/net/wireless/orinoco_pci.c index 5362c21..e57e92b 100644 --- a/drivers/net/wireless/orinoco_pci.c +++ b/drivers/net/wireless/orinoco_pci.c @@ -304,7 +304,9 @@ static int orinoco_pci_suspend(struct pc orinoco_unlock(priv, &flags); + free_irq(pdev->irq, dev); pci_save_state(pdev); + pci_disable_device(pdev); pci_set_power_state(pdev, PCI_D3hot); return 0; @@ -320,7 +322,16 @@ static int orinoco_pci_resume(struct pci printk(KERN_DEBUG "%s: Orinoco-PCI waking up\n", dev->name); pci_set_power_state(pdev, 0); + pci_enable_device(pdev); pci_restore_state(pdev); + + err = request_irq(pdev->irq, orinoco_interrupt, SA_SHIRQ, + dev->name, dev); + if (err) { + printk(KERN_ERR "%s: Cannot re-allocate IRQ\n", dev->name); + pci_disable_device(pdev); + return -EBUSY; + } err = orinoco_reinit_firmware(dev); if (err) { - 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 10/21] orinoco replace hermes_write_words() with hermes_write_bytes()
From: Pavel Roskin <[EMAIL PROTECTED]> The new function can write an odd number of bytes, thus making padding unnecessary. Signed-off-by: Pavel Roskin <[EMAIL PROTECTED]> --- drivers/net/wireless/hermes.c | 17 - drivers/net/wireless/hermes.h |7 +-- drivers/net/wireless/spectrum_cs.c |7 +++ 3 files changed, 16 insertions(+), 15 deletions(-) diff --git a/drivers/net/wireless/hermes.c b/drivers/net/wireless/hermes.c index 456d934..be24f01 100644 --- a/drivers/net/wireless/hermes.c +++ b/drivers/net/wireless/hermes.c @@ -400,8 +400,7 @@ int hermes_bap_pread(hermes_t *hw, int b } /* Write a block of data to the chip's buffer, via the - * BAP. Synchronization/serialization is the caller's problem. len - * must be even. + * BAP. Synchronization/serialization is the caller's problem. * * Returns: < 0 on internal failure (errno), 0 on success, > 0 on error from firmware */ @@ -411,7 +410,7 @@ int hermes_bap_pwrite(hermes_t *hw, int int dreg = bap ? HERMES_DATA1 : HERMES_DATA0; int err = 0; - if ( (len < 0) || (len % 2) ) + if (len < 0) return -EINVAL; err = hermes_bap_seek(hw, bap, id, offset); @@ -419,7 +418,7 @@ int hermes_bap_pwrite(hermes_t *hw, int goto out; /* Actually do the transfer */ - hermes_write_words(hw, dreg, buf, len/2); + hermes_write_bytes(hw, dreg, buf, len); out: return err; @@ -427,7 +426,7 @@ int hermes_bap_pwrite(hermes_t *hw, int /* Write a block of data to the chip's buffer with padding if * neccessary, via the BAP. Synchronization/serialization is the - * caller's problem. len must be even. + * caller's problem. * * Returns: < 0 on internal failure (errno), 0 on success, > 0 on error from firmware */ @@ -437,7 +436,7 @@ int hermes_bap_pwrite_pad(hermes_t *hw, int dreg = bap ? HERMES_DATA1 : HERMES_DATA0; int err = 0; - if (len < 0 || len % 2 || data_len > len) + if (len < 0 || data_len > len) return -EINVAL; err = hermes_bap_seek(hw, bap, id, offset); @@ -445,13 +444,13 @@ int hermes_bap_pwrite_pad(hermes_t *hw, goto out; /* Transfer all the complete words of data */ - hermes_write_words(hw, dreg, buf, data_len/2); + hermes_write_bytes(hw, dreg, buf, data_len); /* If there is an odd byte left over pad and transfer it */ if (data_len & 1) { u8 end[2]; end[1] = 0; end[0] = ((unsigned char *)buf)[data_len - 1]; - hermes_write_words(hw, dreg, end, 1); + hermes_write_bytes(hw, dreg, end, 2); data_len ++; } /* Now send zeros for the padding */ @@ -534,7 +533,7 @@ int hermes_write_ltv(hermes_t *hw, int b count = length - 1; - hermes_write_words(hw, dreg, value, count); + hermes_write_bytes(hw, dreg, value, count << 1); err = hermes_docmd_wait(hw, HERMES_CMD_ACCESS | HERMES_CMD_WRITE, rid, NULL); diff --git a/drivers/net/wireless/hermes.h b/drivers/net/wireless/hermes.h index 34ccba2..e1b279e 100644 --- a/drivers/net/wireless/hermes.h +++ b/drivers/net/wireless/hermes.h @@ -408,10 +408,13 @@ static inline void hermes_read_words(str ioread16_rep(hw->iobase + off, buf, count); } -static inline void hermes_write_words(struct hermes *hw, int off, const void *buf, unsigned count) +static inline void hermes_write_bytes(struct hermes *hw, int off, + const char *buf, unsigned count) { off = off << hw->reg_spacing; - iowrite16_rep(hw->iobase + off, buf, count); + iowrite16_rep(hw->iobase + off, buf, count >> 1); + if (unlikely(count & 1)) + iowrite8(buf[count - 1], hw->iobase + off); } static inline void hermes_clear_words(struct hermes *hw, int off, unsigned count) diff --git a/drivers/net/wireless/spectrum_cs.c b/drivers/net/wireless/spectrum_cs.c index 14f4daa..c13a5b6 100644 --- a/drivers/net/wireless/spectrum_cs.c +++ b/drivers/net/wireless/spectrum_cs.c @@ -343,8 +343,7 @@ spectrum_plug_pdi(hermes_t *hw, struct p /* do the actual plugging */ spectrum_aux_setaddr(hw, pdr_addr(pdr)); - hermes_write_words(hw, HERMES_AUXDATA, pdi->data, - pdi_len(pdi) / 2); + hermes_write_bytes(hw, HERMES_AUXDATA, pdi->data, pdi_len(pdi)); return 0; } @@ -424,8 +423,8 @@ spectrum_load_blocks(hermes_t *hw, const while (dblock_addr(blk) != BLOCK_END) { spectrum_aux_setaddr(hw, blkaddr); - hermes_write_words(hw, HERMES_AUXDATA, blk->data, - blklen / 2); + hermes_write_bytes(hw, HERMES_AUXDATA, blk->data, + blklen); blk = (struct dblock *) &blk->data[blklen];
[PATCH 11/21] orinoco: don't use any padding for Tx frames
From: Pavel Roskin <[EMAIL PROTECTED]> hermes_bap_pwrite() supports odd-sized packets now. There is no minimal packet size for 802.11. Also, hermes_bap_pwrite() supports odd-sized packets now. This removes all reasons to pad the Tx data. Signed-off-by: Pavel Roskin <[EMAIL PROTECTED]> --- drivers/net/wireless/hermes.c | 38 -- drivers/net/wireless/hermes.h |2 -- drivers/net/wireless/orinoco.c | 25 +++-- 3 files changed, 7 insertions(+), 58 deletions(-) diff --git a/drivers/net/wireless/hermes.c b/drivers/net/wireless/hermes.c index be24f01..2aa2f38 100644 --- a/drivers/net/wireless/hermes.c +++ b/drivers/net/wireless/hermes.c @@ -424,43 +424,6 @@ int hermes_bap_pwrite(hermes_t *hw, int return err; } -/* Write a block of data to the chip's buffer with padding if - * neccessary, via the BAP. Synchronization/serialization is the - * caller's problem. - * - * Returns: < 0 on internal failure (errno), 0 on success, > 0 on error from firmware - */ -int hermes_bap_pwrite_pad(hermes_t *hw, int bap, const void *buf, unsigned data_len, int len, - u16 id, u16 offset) -{ - int dreg = bap ? HERMES_DATA1 : HERMES_DATA0; - int err = 0; - - if (len < 0 || data_len > len) - return -EINVAL; - - err = hermes_bap_seek(hw, bap, id, offset); - if (err) - goto out; - - /* Transfer all the complete words of data */ - hermes_write_bytes(hw, dreg, buf, data_len); - /* If there is an odd byte left over pad and transfer it */ - if (data_len & 1) { - u8 end[2]; - end[1] = 0; - end[0] = ((unsigned char *)buf)[data_len - 1]; - hermes_write_bytes(hw, dreg, end, 2); - data_len ++; - } - /* Now send zeros for the padding */ - if (data_len < len) - hermes_clear_words(hw, dreg, (len - data_len) / 2); - /* Complete */ - out: - return err; -} - /* Read a Length-Type-Value record from the card. * * If length is NULL, we ignore the length read from the card, and @@ -548,7 +511,6 @@ EXPORT_SYMBOL(hermes_allocate); EXPORT_SYMBOL(hermes_bap_pread); EXPORT_SYMBOL(hermes_bap_pwrite); -EXPORT_SYMBOL(hermes_bap_pwrite_pad); EXPORT_SYMBOL(hermes_read_ltv); EXPORT_SYMBOL(hermes_write_ltv); diff --git a/drivers/net/wireless/hermes.h b/drivers/net/wireless/hermes.h index e1b279e..8e3f0e3 100644 --- a/drivers/net/wireless/hermes.h +++ b/drivers/net/wireless/hermes.h @@ -359,8 +359,6 @@ int hermes_bap_pread(hermes_t *hw, int b u16 id, u16 offset); int hermes_bap_pwrite(hermes_t *hw, int bap, const void *buf, int len, u16 id, u16 offset); -int hermes_bap_pwrite_pad(hermes_t *hw, int bap, const void *buf, - unsigned data_len, int len, u16 id, u16 offset); int hermes_read_ltv(hermes_t *hw, int bap, u16 rid, unsigned buflen, u16 *length, void *buf); int hermes_write_ltv(hermes_t *hw, int bap, u16 rid, diff --git a/drivers/net/wireless/orinoco.c b/drivers/net/wireless/orinoco.c index 173e9e4..9fde17b 100644 --- a/drivers/net/wireless/orinoco.c +++ b/drivers/net/wireless/orinoco.c @@ -423,7 +423,7 @@ static int orinoco_xmit(struct sk_buff * u16 txfid = priv->txfid; char *p; struct ethhdr *eh; - int len, data_len, data_off; + int data_len, data_off; struct hermes_tx_descriptor desc; unsigned long flags; @@ -455,13 +455,10 @@ static int orinoco_xmit(struct sk_buff * return NETDEV_TX_OK; } - /* Length of the packet body */ - /* FIXME: what if the skb is smaller than this? */ - len = max_t(int, ALIGN(skb->len, 2), ETH_ZLEN); - skb = skb_padto(skb, len); - if (skb == NULL) + /* Check packet length */ + data_len = skb->len; + if (data_len < ETH_HLEN) goto fail; - len -= ETH_HLEN; eh = (struct ethhdr *)skb->data; @@ -485,7 +482,7 @@ static int orinoco_xmit(struct sk_buff * /* Encapsulate Ethernet-II frames */ if (ntohs(eh->h_proto) > ETH_DATA_LEN) { /* Ethernet-II frame */ struct header_struct hdr; - data_len = len; + data_len = skb->len - ETH_HLEN; data_off = HERMES_802_3_OFFSET + sizeof(hdr); p = skb->data + ETH_HLEN; @@ -507,21 +504,13 @@ static int orinoco_xmit(struct sk_buff * stats->tx_errors++; goto fail; } - /* Actual xfer length - allow for padding */ - len = ALIGN(data_len, 2); - if (len < ETH_ZLEN - ETH_HLEN) - len = ETH_ZLEN - ETH_HLEN; } else { /* IEEE 802.3 frame */ - data_len = len + ETH_HLEN; + data_len = skb->len; data_off
[PATCH 19/21] orinoco: reduce differences between PCI drivers, create orinoco_pci.h
From: Pavel Roskin <[EMAIL PROTECTED]> Make all Orinoco PCI drivers (orinoco_pci, orinoco_plx, orinoco_tmd and orinoco_nortel) as similar as possible. Use the best implementation of error handling, the best error messages, the best comments. Put common code to orinoco_pci.h. For now, it's suspend and resume functions and function for registering the network device. Signed-off-by: Pavel Roskin <[EMAIL PROTECTED]> --- drivers/net/wireless/orinoco_nortel.c | 240 +++ drivers/net/wireless/orinoco_pci.c| 144 +++--- drivers/net/wireless/orinoco_pci.h| 125 drivers/net/wireless/orinoco_plx.c| 259 +++-- drivers/net/wireless/orinoco_tmd.c| 165 + 5 files changed, 368 insertions(+), 565 deletions(-) diff --git a/drivers/net/wireless/orinoco_nortel.c b/drivers/net/wireless/orinoco_nortel.c index 3fff013..deb22fb 100644 --- a/drivers/net/wireless/orinoco_nortel.c +++ b/drivers/net/wireless/orinoco_nortel.c @@ -1,5 +1,5 @@ /* orinoco_nortel.c - * + * * Driver for Prism II devices which would usually be driven by orinoco_cs, * but are connected to the PCI bus by a PCI-to-PCMCIA adapter used in * Nortel emobility, Symbol LA-4113 and Symbol LA-4123. @@ -50,16 +50,11 @@ #include #include #include "orinoco.h" +#include "orinoco_pci.h" #define COR_OFFSET(0xe0) /* COR attribute offset of Prism2 PC card */ #define COR_VALUE (COR_LEVEL_REQ | COR_FUNC_ENA) /* Enable PC card with interrupt in level trigger */ - -/* Nortel specific data */ -struct nortel_pci_card { - unsigned long iobase1; - unsigned long iobase2; -}; /* * Do a soft reset of the PCI card using the Configuration Option Register @@ -69,48 +64,48 @@ struct nortel_pci_card { * Note bis : Don't try to access HERMES_CMD during the reset phase. * It just won't work ! */ -static int nortel_pci_cor_reset(struct orinoco_private *priv) +static int orinoco_nortel_cor_reset(struct orinoco_private *priv) { - struct nortel_pci_card *card = priv->card; + struct orinoco_pci_card *card = priv->card; /* Assert the reset until the card notice */ - outw_p(8, card->iobase1 + 2); - inw(card->iobase2 + COR_OFFSET); - outw_p(0x80, card->iobase2 + COR_OFFSET); + iowrite16(8, card->bridge_io + 2); + ioread16(card->attr_io + COR_OFFSET); + iowrite16(0x80, card->attr_io + COR_OFFSET); mdelay(1); /* Give time for the card to recover from this hard effort */ - outw_p(0, card->iobase2 + COR_OFFSET); - outw_p(0, card->iobase2 + COR_OFFSET); + iowrite16(0, card->attr_io + COR_OFFSET); + iowrite16(0, card->attr_io + COR_OFFSET); mdelay(1); - /* set COR as usual */ - outw_p(COR_VALUE, card->iobase2 + COR_OFFSET); - outw_p(COR_VALUE, card->iobase2 + COR_OFFSET); + /* Set COR as usual */ + iowrite16(COR_VALUE, card->attr_io + COR_OFFSET); + iowrite16(COR_VALUE, card->attr_io + COR_OFFSET); mdelay(1); - outw_p(0x228, card->iobase1 + 2); + iowrite16(0x228, card->bridge_io + 2); return 0; } -static int nortel_pci_hw_init(struct nortel_pci_card *card) +static int orinoco_nortel_hw_init(struct orinoco_pci_card *card) { int i; u32 reg; - /* setup bridge */ - if (inw(card->iobase1) & 1) { + /* Setup bridge */ + if (ioread16(card->bridge_io) & 1) { printk(KERN_ERR PFX "brg1 answer1 wrong\n"); return -EBUSY; } - outw_p(0x118, card->iobase1 + 2); - outw_p(0x108, card->iobase1 + 2); + iowrite16(0x118, card->bridge_io + 2); + iowrite16(0x108, card->bridge_io + 2); mdelay(30); - outw_p(0x8, card->iobase1 + 2); + iowrite16(0x8, card->bridge_io + 2); for (i = 0; i < 30; i++) { mdelay(30); - if (inw(card->iobase1) & 0x10) { + if (ioread16(card->bridge_io) & 0x10) { break; } } @@ -118,42 +113,42 @@ static int nortel_pci_hw_init(struct nor printk(KERN_ERR PFX "brg1 timed out\n"); return -EBUSY; } - if (inw(card->iobase2 + 0xe0) & 1) { + if (ioread16(card->attr_io + COR_OFFSET) & 1) { printk(KERN_ERR PFX "brg2 answer1 wrong\n"); return -EBUSY; } - if (inw(card->iobase2 + 0xe2) & 1) { + if (ioread16(card->attr_io + COR_OFFSET + 2) & 1) { printk(KERN_ERR PFX "brg2 answer2 wrong\n"); return -EBUSY; } - if (inw(card->iobase2 + 0xe4) & 1) { + if (ioread16(card->attr_io + COR_OFFSET + 4) & 1) { printk(KERN_ERR PFX "brg2 answer3 wrong\n"); return -EBUSY; } - /* set the PCMCIA COR-Register */ - outw_p(COR_VALUE, card->iobase2 + COR
[PATCH 15/21] orinoco: delay FID allocation after firmware initialization
From: Pavel Roskin <[EMAIL PROTECTED]> This is needed to identify the card before possible allocation problems, so that the user at least can report the firmware version that fails. Signed-off-by: Pavel Roskin <[EMAIL PROTECTED]> --- drivers/net/wireless/orinoco.c | 28 ++-- 1 files changed, 22 insertions(+), 6 deletions(-) diff --git a/drivers/net/wireless/orinoco.c b/drivers/net/wireless/orinoco.c index 80cf6fb..d4c13ff 100644 --- a/drivers/net/wireless/orinoco.c +++ b/drivers/net/wireless/orinoco.c @@ -1345,15 +1345,11 @@ int __orinoco_down(struct net_device *de return 0; } -int orinoco_reinit_firmware(struct net_device *dev) +static int orinoco_allocate_fid(struct net_device *dev) { struct orinoco_private *priv = netdev_priv(dev); struct hermes *hw = &priv->hw; int err; - - err = hermes_init(hw); - if (err) - return err; err = hermes_allocate(hw, priv->nicbuf_size, &priv->txfid); if (err == -EIO && priv->nicbuf_size > TX_NICBUF_SIZE_BUG) { @@ -1372,7 +1368,20 @@ int orinoco_reinit_firmware(struct net_d return err; } + +int orinoco_reinit_firmware(struct net_device *dev) +{ + struct orinoco_private *priv = netdev_priv(dev); + struct hermes *hw = &priv->hw; + int err; + err = hermes_init(hw); + if (!err) + err = orinoco_allocate_fid(dev); + + return err; +} + static int __orinoco_hw_set_bitrate(struct orinoco_private *priv) { hermes_t *hw = &priv->hw; @@ -2224,7 +2233,7 @@ static int orinoco_init(struct net_devic priv->nicbuf_size = IEEE80211_FRAME_LEN + ETH_HLEN; /* Initialize the firmware */ - err = orinoco_reinit_firmware(dev); + err = hermes_init(hw); if (err != 0) { printk(KERN_ERR "%s: failed to initialize firmware (err = %d)\n", dev->name, err); @@ -2281,6 +2290,13 @@ static int orinoco_init(struct net_devic priv->nick[len] = '\0'; printk(KERN_DEBUG "%s: Station name \"%s\"\n", dev->name, priv->nick); + + err = orinoco_allocate_fid(dev); + if (err) { + printk(KERN_ERR "%s: failed to allocate NIC buffer!\n", + dev->name); + goto out; + } /* Get allowed channels */ err = hermes_read_wordrec(hw, USER_BAP, HERMES_RID_CHANNELLIST, - 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 09/21] orinoco: orinoco_xmit() should only return valid symbolic constants
From: Pavel Roskin <[EMAIL PROTECTED]> Don't ever return -errno from orinoco_xmit() - the network layer doesn't expect it. Signed-off-by: Pavel Roskin <[EMAIL PROTECTED]> --- drivers/net/wireless/orinoco.c | 12 ++-- 1 files changed, 6 insertions(+), 6 deletions(-) diff --git a/drivers/net/wireless/orinoco.c b/drivers/net/wireless/orinoco.c index ceea494..173e9e4 100644 --- a/drivers/net/wireless/orinoco.c +++ b/drivers/net/wireless/orinoco.c @@ -430,19 +430,19 @@ static int orinoco_xmit(struct sk_buff * if (! netif_running(dev)) { printk(KERN_ERR "%s: Tx on stopped device!\n", dev->name); - return 1; + return NETDEV_TX_BUSY; } if (netif_queue_stopped(dev)) { printk(KERN_DEBUG "%s: Tx while transmitter busy!\n", dev->name); - return 1; + return NETDEV_TX_BUSY; } if (orinoco_lock(priv, &flags) != 0) { printk(KERN_ERR "%s: orinoco_xmit() called while hw_unavailable\n", dev->name); - return 1; + return NETDEV_TX_BUSY; } if (! netif_carrier_ok(dev) || (priv->iw_mode == IW_MODE_MONITOR)) { @@ -452,7 +452,7 @@ static int orinoco_xmit(struct sk_buff * stats->tx_errors++; orinoco_unlock(priv, &flags); dev_kfree_skb(skb); - return 0; + return NETDEV_TX_OK; } /* Length of the packet body */ @@ -551,11 +551,11 @@ static int orinoco_xmit(struct sk_buff * dev_kfree_skb(skb); - return 0; + return NETDEV_TX_OK; fail: orinoco_unlock(priv, &flags); - return err; + return NETDEV_TX_BUSY; } static void __orinoco_ev_alloc(struct net_device *dev, hermes_t *hw) - 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 20/21] orinoco: further comment cleanup in the PCI drivers
From: Pavel Roskin <[EMAIL PROTECTED]> Signed-off-by: Pavel Roskin <[EMAIL PROTECTED]> --- drivers/net/wireless/orinoco_nortel.c |7 +-- drivers/net/wireless/orinoco_pci.c| 69 + drivers/net/wireless/orinoco_plx.c| 42 +++- drivers/net/wireless/orinoco_tmd.c| 16 +--- 4 files changed, 22 insertions(+), 112 deletions(-) diff --git a/drivers/net/wireless/orinoco_nortel.c b/drivers/net/wireless/orinoco_nortel.c index deb22fb..1596182 100644 --- a/drivers/net/wireless/orinoco_nortel.c +++ b/drivers/net/wireless/orinoco_nortel.c @@ -3,7 +3,6 @@ * Driver for Prism II devices which would usually be driven by orinoco_cs, * but are connected to the PCI bus by a PCI-to-PCMCIA adapter used in * Nortel emobility, Symbol LA-4113 and Symbol LA-4123. - * but are connected to the PCI bus by a Nortel PCI-PCMCIA-Adapter. * * Copyright (C) 2002 Tobias Hoffmann * (C) 2003 Christoph Jungegger <[EMAIL PROTECTED]> @@ -57,7 +56,7 @@ #define COR_VALUE (COR_LEVEL_REQ | C /* - * Do a soft reset of the PCI card using the Configuration Option Register + * Do a soft reset of the card using the Configuration Option Register * We need this to get going... * This is the part of the code that is strongly inspired from wlan-ng * @@ -68,7 +67,7 @@ static int orinoco_nortel_cor_reset(stru { struct orinoco_pci_card *card = priv->card; - /* Assert the reset until the card notice */ + /* Assert the reset until the card notices */ iowrite16(8, card->bridge_io + 2); ioread16(card->attr_io + COR_OFFSET); iowrite16(0x80, card->attr_io + COR_OFFSET); @@ -126,7 +125,7 @@ static int orinoco_nortel_hw_init(struct return -EBUSY; } - /* Set the PCMCIA COR-Register */ + /* Set the PCMCIA COR register */ iowrite16(COR_VALUE, card->attr_io + COR_OFFSET); mdelay(1); reg = ioread16(card->attr_io + COR_OFFSET); diff --git a/drivers/net/wireless/orinoco_pci.c b/drivers/net/wireless/orinoco_pci.c index 41efac2..df37b95 100644 --- a/drivers/net/wireless/orinoco_pci.c +++ b/drivers/net/wireless/orinoco_pci.c @@ -1,11 +1,11 @@ /* orinoco_pci.c * - * Driver for Prism II devices that have a direct PCI interface - * (i.e., not in a Pcmcia or PLX bridge) - * - * Specifically here we're talking about the Linksys WMP11 + * Driver for Prism 2.5/3 devices that have a direct PCI interface + * (i.e. these are not PCMCIA cards in a PCMCIA-to-PCI bridge). + * The card contains only one PCI region, which contains all the usual + * hermes registers, as well as the COR register. * - * Current maintainers (as of 29 September 2003) are: + * Current maintainers are: * Pavel Roskin * and David Gibson * @@ -39,54 +39,6 @@ * other provisions required by the GPL. If you do not delete the * provisions above, a recipient may use your version of this file * under either the MPL or the GPL. - */ - -/* - * Theory of operation... - * --- - * Maybe you had a look in orinoco_plx. Well, this is totally different... - * - * The card contains only one PCI region, which contains all the usual - * hermes registers. - * - * The driver will memory map this region in normal memory. Because - * the hermes registers are mapped in normal memory and not in ISA I/O - * post space, we can't use the usual inw/outw macros and we need to - * use readw/writew. - * This slight difference force us to compile our own version of - * hermes.c with the register access macro changed. That's a bit - * hackish but works fine. - * - * Note that the PCI region is pretty big (4K). That's much more than - * the usual set of hermes register (0x0 -> 0x3E). I've got a strong - * suspicion that the whole memory space of the adapter is in fact in - * this region. Accessing directly the adapter memory instead of going - * through the usual register would speed up significantely the - * operations... - * - * Finally, the card looks like this : - Bus 0, device 14, function 0: -Network controller: PCI device 1260:3873 (Harris Semiconductor) (rev 1). - IRQ 11. - Master Capable. Latency=248. - Prefetchable 32 bit memory at 0xffbcc000 [0xffbccfff]. -00:0e.0 Network controller: Harris Semiconductor: Unknown device 3873 (rev 01) -Subsystem: Unknown device 1737:3874 -Control: I/O+ Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR- FastB2B- -Status: Cap+ 66Mhz- UDF- FastB2B+ ParErr- DEVSEL=medium >TAbort- SERR- #include "orinoco.h" #include "orinoco_pci.h" -/* All the magic there is from wlan-ng */ -/* Magic offset of the reset register of the PCI card */ +/* Offset of the COR register of the PCI card */ #define HERMES_PCI_COR (0x26) -/* Magic bitmask to reset the card */ + +/* Bitmask to reset the card */ #define HERMES_PCI_COR_MASK(0x0080)
[PATCH 17/21] orinoco_pci: use pci_iomap() for resources
From: Pavel Roskin <[EMAIL PROTECTED]> Signed-off-by: Pavel Roskin <[EMAIL PROTECTED]> --- drivers/net/wireless/orinoco_pci.c | 15 ++- 1 files changed, 6 insertions(+), 9 deletions(-) diff --git a/drivers/net/wireless/orinoco_pci.c b/drivers/net/wireless/orinoco_pci.c index e57e92b..75df90f 100644 --- a/drivers/net/wireless/orinoco_pci.c +++ b/drivers/net/wireless/orinoco_pci.c @@ -170,9 +170,7 @@ static int orinoco_pci_init_one(struct p const struct pci_device_id *ent) { int err = 0; - unsigned long pci_iorange; - u16 __iomem *pci_ioaddr = NULL; - unsigned long pci_iolen; + void __iomem *pci_ioaddr = NULL; struct orinoco_private *priv = NULL; struct orinoco_pci_card *card; struct net_device *dev = NULL; @@ -190,10 +188,9 @@ static int orinoco_pci_init_one(struct p } /* Resource 0 is mapped to the hermes registers */ - pci_iorange = pci_resource_start(pdev, 0); - pci_iolen = pci_resource_len(pdev, 0); - pci_ioaddr = ioremap(pci_iorange, pci_iolen); - if (!pci_iorange) { + pci_ioaddr = pci_iomap(pdev, 0, 0); + if (!pci_ioaddr) { + err = -EIO; printk(KERN_ERR PFX "Cannot remap hardware registers\n"); goto fail_map; } @@ -208,8 +205,8 @@ static int orinoco_pci_init_one(struct p priv = netdev_priv(dev); card = priv->card; card->pci_ioaddr = pci_ioaddr; - dev->mem_start = pci_iorange; - dev->mem_end = pci_iorange + pci_iolen - 1; + dev->mem_start = pci_resource_start(pdev, 0); + dev->mem_end = dev->mem_start + pci_resource_len(pdev, 0) - 1; SET_MODULE_OWNER(dev); SET_NETDEV_DEV(dev, &pdev->dev); - 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 01/21] orinoco: Remove useless CIS validation
From: Pavel Roskin <[EMAIL PROTECTED]> The PCMCIA drivers would never be loaded if the CIS were wrong. No other PCMCIA drivers validate CIS. Signed-off-by: Pavel Roskin <[EMAIL PROTECTED]> --- drivers/net/wireless/orinoco_cs.c |3 --- drivers/net/wireless/spectrum_cs.c |3 --- 2 files changed, 0 insertions(+), 6 deletions(-) diff --git a/drivers/net/wireless/orinoco_cs.c b/drivers/net/wireless/orinoco_cs.c index ec6f2a4..05f1762 100644 --- a/drivers/net/wireless/orinoco_cs.c +++ b/drivers/net/wireless/orinoco_cs.c @@ -191,12 +191,9 @@ orinoco_cs_config(dev_link_t *link) int last_fn, last_ret; u_char buf[64]; config_info_t conf; - cisinfo_t info; tuple_t tuple; cisparse_t parse; void __iomem *mem; - - CS_CHECK(ValidateCIS, pcmcia_validate_cis(handle, &info)); /* * This reads the card's CONFIG tuple to find its diff --git a/drivers/net/wireless/spectrum_cs.c b/drivers/net/wireless/spectrum_cs.c index 5fa6fbe..87178a3 100644 --- a/drivers/net/wireless/spectrum_cs.c +++ b/drivers/net/wireless/spectrum_cs.c @@ -664,12 +664,9 @@ spectrum_cs_config(dev_link_t *link) int last_fn, last_ret; u_char buf[64]; config_info_t conf; - cisinfo_t info; tuple_t tuple; cisparse_t parse; void __iomem *mem; - - CS_CHECK(ValidateCIS, pcmcia_validate_cis(handle, &info)); /* * This reads the card's CONFIG tuple to find its - 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 05/21] orinoco: remove tracing code, it's unused
From: Pavel Roskin <[EMAIL PROTECTED]> Signed-off-by: Pavel Roskin <[EMAIL PROTECTED]> --- drivers/net/wireless/orinoco.c | 83 drivers/net/wireless/orinoco.h |3 - 2 files changed, 0 insertions(+), 86 deletions(-) diff --git a/drivers/net/wireless/orinoco.c b/drivers/net/wireless/orinoco.c index 06523e2..9550bf3 100644 --- a/drivers/net/wireless/orinoco.c +++ b/drivers/net/wireless/orinoco.c @@ -456,26 +456,21 @@ static int orinoco_xmit(struct sk_buff * struct hermes_tx_descriptor desc; unsigned long flags; - TRACE_ENTER(dev->name); - if (! netif_running(dev)) { printk(KERN_ERR "%s: Tx on stopped device!\n", dev->name); - TRACE_EXIT(dev->name); return 1; } if (netif_queue_stopped(dev)) { printk(KERN_DEBUG "%s: Tx while transmitter busy!\n", dev->name); - TRACE_EXIT(dev->name); return 1; } if (orinoco_lock(priv, &flags) != 0) { printk(KERN_ERR "%s: orinoco_xmit() called while hw_unavailable\n", dev->name); - TRACE_EXIT(dev->name); return 1; } @@ -486,7 +481,6 @@ static int orinoco_xmit(struct sk_buff * stats->tx_errors++; orinoco_unlock(priv, &flags); dev_kfree_skb(skb); - TRACE_EXIT(dev->name); return 0; } @@ -585,12 +579,9 @@ static int orinoco_xmit(struct sk_buff * orinoco_unlock(priv, &flags); dev_kfree_skb(skb); - - TRACE_EXIT(dev->name); return 0; fail: - TRACE_EXIT(dev->name); orinoco_unlock(priv, &flags); return err; @@ -2274,8 +2265,6 @@ static int orinoco_init(struct net_devic u16 reclen; int len; - TRACE_ENTER(dev->name); - /* No need to lock, the hw_unavailable flag is already set in * alloc_orinocodev() */ priv->nicbuf_size = IEEE80211_FRAME_LEN + ETH_HLEN; @@ -2429,7 +2418,6 @@ static int orinoco_init(struct net_devic printk(KERN_DEBUG "%s: ready\n", dev->name); out: - TRACE_EXIT(dev->name); return err; } @@ -2796,8 +2784,6 @@ static int orinoco_ioctl_getiwrange(stru struct iw_range *range = (struct iw_range *) extra; int numrates; int i, k; - - TRACE_ENTER(dev->name); rrq->length = sizeof(struct iw_range); memset(range, 0, sizeof(struct iw_range)); @@ -2888,8 +2874,6 @@ static int orinoco_ioctl_getiwrange(stru IW_EVENT_CAPA_SET(range->event_capa, SIOCGIWSCAN); IW_EVENT_CAPA_SET(range->event_capa, IWEVTXDROP); - TRACE_EXIT(dev->name); - return 0; } @@ -3070,8 +3054,6 @@ static int orinoco_ioctl_getessid(struct int active; int err = 0; unsigned long flags; - - TRACE_ENTER(dev->name); if (netif_running(dev)) { err = orinoco_hw_get_essid(priv, &active, essidbuf); @@ -3087,8 +3069,6 @@ static int orinoco_ioctl_getessid(struct erq->flags = 1; erq->length = strlen(essidbuf) + 1; - TRACE_EXIT(dev->name); - return 0; } @@ -4347,69 +4327,6 @@ static struct ethtool_ops orinoco_ethtoo .get_drvinfo = orinoco_get_drvinfo, .get_link = ethtool_op_get_link, }; - -// -/* Debugging*/ -// - -#if 0 -static void show_rx_frame(struct orinoco_rxframe_hdr *frame) -{ - printk(KERN_DEBUG "RX descriptor:\n"); - printk(KERN_DEBUG " status = 0x%04x\n", frame->desc.status); - printk(KERN_DEBUG " time= 0x%08x\n", frame->desc.time); - printk(KERN_DEBUG " silence = 0x%02x\n", frame->desc.silence); - printk(KERN_DEBUG " signal = 0x%02x\n", frame->desc.signal); - printk(KERN_DEBUG " rate= 0x%02x\n", frame->desc.rate); - printk(KERN_DEBUG " rxflow = 0x%02x\n", frame->desc.rxflow); - printk(KERN_DEBUG " reserved= 0x%08x\n", frame->desc.reserved); - - printk(KERN_DEBUG "IEEE 802.11 header:\n"); - printk(KERN_DEBUG " frame_ctl = 0x%04x\n", - frame->p80211.frame_ctl); - printk(KERN_DEBUG " duration_id = 0x%04x\n", - frame->p80211.duration_id); - printk(KERN_DEBUG " addr1 = %02x:%02x:%02x:%02x:%02x:%02x\n", - frame->p80211.addr1[0], frame->p80211.addr1[1], - frame->p80211.addr1[2], frame->p80211.addr1[3], - frame->p80211.addr1[4], frame->p80211.addr1[5]); - printk(KERN_DEBUG " addr2 = %02x:%02x:%02x:%02x:%02x:%02x\n", - frame->p80211.addr2[0], frame->p80211.addr2[1],
[PATCH 04/21] orinoco: fix truncating commsquality RID with the latest Symbol firmware
From: Pavel Roskin <[EMAIL PROTECTED]> Symbol firmware F3.91-71 has an additional word in the commsquality RID. Extend the receiving buffer by one word to accomodate it. Signed-off-by: Pavel Roskin <[EMAIL PROTECTED]> --- drivers/net/wireless/orinoco.c |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/drivers/net/wireless/orinoco.c b/drivers/net/wireless/orinoco.c index 8dfdfbd..06523e2 100644 --- a/drivers/net/wireless/orinoco.c +++ b/drivers/net/wireless/orinoco.c @@ -390,7 +390,7 @@ static struct iw_statistics *orinoco_get } } else { struct { - __le16 qual, signal, noise; + __le16 qual, signal, noise, unused; } __attribute__ ((packed)) cq; err = HERMES_READ_RECORD(hw, USER_BAP, - 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 06/21] orinoco: remove debug buffer code and userspace include support
From: Pavel Roskin <[EMAIL PROTECTED]> Signed-off-by: Pavel Roskin <[EMAIL PROTECTED]> --- drivers/net/wireless/hermes.c | 19 --- drivers/net/wireless/hermes.h | 34 -- 2 files changed, 0 insertions(+), 53 deletions(-) diff --git a/drivers/net/wireless/hermes.c b/drivers/net/wireless/hermes.c index 346c6fe..456d934 100644 --- a/drivers/net/wireless/hermes.c +++ b/drivers/net/wireless/hermes.c @@ -121,12 +121,6 @@ void hermes_struct_init(hermes_t *hw, vo hw->iobase = address; hw->reg_spacing = reg_spacing; hw->inten = 0x0; - -#ifdef HERMES_DEBUG_BUFFER - hw->dbufp = 0; - memset(&hw->dbuf, 0xff, sizeof(hw->dbuf)); - memset(&hw->profile, 0, sizeof(hw->profile)); -#endif } int hermes_init(hermes_t *hw) @@ -345,20 +339,7 @@ static int hermes_bap_seek(hermes_t *hw, k--; udelay(1); reg = hermes_read_reg(hw, oreg); - } - -#ifdef HERMES_DEBUG_BUFFER - hw->profile[HERMES_BAP_BUSY_TIMEOUT - k]++; - - if (k < HERMES_BAP_BUSY_TIMEOUT) { - struct hermes_debug_entry *e = - &hw->dbuf[(hw->dbufp++) % HERMES_DEBUG_BUFSIZE]; - e->bap = bap; - e->id = id; - e->offset = offset; - e->cycles = HERMES_BAP_BUSY_TIMEOUT - k; } -#endif if (reg & HERMES_OFFSET_BUSY) return -ETIMEDOUT; diff --git a/drivers/net/wireless/hermes.h b/drivers/net/wireless/hermes.h index 7644f72..34ccba2 100644 --- a/drivers/net/wireless/hermes.h +++ b/drivers/net/wireless/hermes.h @@ -328,16 +328,6 @@ struct hermes_multicast { u8 addr[HERMES_MAX_MULTICAST][ETH_ALEN]; } __attribute__ ((packed)); -// #define HERMES_DEBUG_BUFFER 1 -#define HERMES_DEBUG_BUFSIZE 4096 -struct hermes_debug_entry { - int bap; - u16 id, offset; - int cycles; -}; - -#ifdef __KERNEL__ - /* Timeouts */ #define HERMES_BAP_BUSY_TIMEOUT (1) /* In iterations of ~1us */ @@ -347,14 +337,7 @@ typedef struct hermes { int reg_spacing; #define HERMES_16BIT_REGSPACING0 #define HERMES_32BIT_REGSPACING1 - u16 inten; /* Which interrupts should be enabled? */ - -#ifdef HERMES_DEBUG_BUFFER - struct hermes_debug_entry dbuf[HERMES_DEBUG_BUFSIZE]; - unsigned long dbufp; - unsigned long profile[HERMES_BAP_BUSY_TIMEOUT+1]; -#endif } hermes_t; /* Register access convenience macros */ @@ -461,22 +444,5 @@ static inline int hermes_write_wordrec(h __le16 rec = cpu_to_le16(word); return HERMES_WRITE_RECORD(hw, bap, rid, &rec); } - -#else /* ! __KERNEL__ */ - -/* These are provided for the benefit of userspace drivers and testing programs - which use ioperm() or iopl() */ - -#define hermes_read_reg(base, off) (inw((base) + (off))) -#define hermes_write_reg(base, off, val) (outw((val), (base) + (off))) - -#define hermes_read_regn(base, name) (hermes_read_reg((base), HERMES_##name)) -#define hermes_write_regn(base, name, val) (hermes_write_reg((base), HERMES_##name, (val))) - -/* Note that for the next two, the count is in 16-bit words, not bytes */ -#define hermes_read_data(base, off, buf, count) (insw((base) + (off), (buf), (count))) -#define hermes_write_data(base, off, buf, count) (outsw((base) + (off), (buf), (count))) - -#endif /* ! __KERNEL__ */ #endif /* _HERMES_H */ - 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 02/21] orinoco: remove PCMCIA audio support, it's useless for wireless cards
From: Pavel Roskin <[EMAIL PROTECTED]> Signed-off-by: Pavel Roskin <[EMAIL PROTECTED]> --- drivers/net/wireless/orinoco_cs.c |6 -- drivers/net/wireless/spectrum_cs.c |6 -- 2 files changed, 0 insertions(+), 12 deletions(-) diff --git a/drivers/net/wireless/orinoco_cs.c b/drivers/net/wireless/orinoco_cs.c index 05f1762..eda01a7 100644 --- a/drivers/net/wireless/orinoco_cs.c +++ b/drivers/net/wireless/orinoco_cs.c @@ -248,12 +248,6 @@ orinoco_cs_config(dev_link_t *link) goto next_entry; link->conf.ConfigIndex = cfg->index; - /* Does this card need audio output? */ - if (cfg->flags & CISTPL_CFTABLE_AUDIO) { - link->conf.Attributes |= CONF_ENABLE_SPKR; - link->conf.Status = CCSR_AUDIO_ENA; - } - /* Use power settings for Vcc and Vpp if present */ /* Note that the CIS values need to be rescaled */ if (cfg->vcc.present & (1 << CISTPL_POWER_VNOM)) { diff --git a/drivers/net/wireless/spectrum_cs.c b/drivers/net/wireless/spectrum_cs.c index 87178a3..0b0bce7 100644 --- a/drivers/net/wireless/spectrum_cs.c +++ b/drivers/net/wireless/spectrum_cs.c @@ -721,12 +721,6 @@ spectrum_cs_config(dev_link_t *link) goto next_entry; link->conf.ConfigIndex = cfg->index; - /* Does this card need audio output? */ - if (cfg->flags & CISTPL_CFTABLE_AUDIO) { - link->conf.Attributes |= CONF_ENABLE_SPKR; - link->conf.Status = CCSR_AUDIO_ENA; - } - /* Use power settings for Vcc and Vpp if present */ /* Note that the CIS values need to be rescaled */ if (cfg->vcc.present & (1 << CISTPL_POWER_VNOM)) { - 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 03/21] orinoco: remove underscores from little-endian field names
From: Pavel Roskin <[EMAIL PROTECTED]> Sparse is much better at finding endianess issues than such visual cues. Signed-off-by: Pavel Roskin <[EMAIL PROTECTED]> --- drivers/net/wireless/spectrum_cs.c | 28 ++-- 1 files changed, 14 insertions(+), 14 deletions(-) diff --git a/drivers/net/wireless/spectrum_cs.c b/drivers/net/wireless/spectrum_cs.c index 0b0bce7..58c153a 100644 --- a/drivers/net/wireless/spectrum_cs.c +++ b/drivers/net/wireless/spectrum_cs.c @@ -120,8 +120,8 @@ #define TEXT_END0x1A/* End of text he * Each block has the following structure. */ struct dblock { - __le32 _addr; /* adapter address where to write the block */ - __le16 _len;/* length of the data only, in bytes */ + __le32 addr;/* adapter address where to write the block */ + __le16 len; /* length of the data only, in bytes */ char data[0]; /* data to be written */ } __attribute__ ((packed)); @@ -131,9 +131,9 @@ struct dblock { * items with matching ID should be written. */ struct pdr { - __le32 _id; /* record ID */ - __le32 _addr; /* adapter address where to write the data */ - __le32 _len;/* expected length of the data, in bytes */ + __le32 id; /* record ID */ + __le32 addr;/* adapter address where to write the data */ + __le32 len; /* expected length of the data, in bytes */ char next[0]; /* next PDR starts here */ } __attribute__ ((packed)); @@ -144,8 +144,8 @@ struct pdr { * be plugged into the secondary firmware. */ struct pdi { - __le16 _len;/* length of ID and data, in words */ - __le16 _id; /* record ID */ + __le16 len; /* length of ID and data, in words */ + __le16 id; /* record ID */ char data[0]; /* plug data */ } __attribute__ ((packed)); @@ -154,44 +154,44 @@ struct pdi { static inline u32 dblock_addr(const struct dblock *blk) { - return le32_to_cpu(blk->_addr); + return le32_to_cpu(blk->addr); } static inline u32 dblock_len(const struct dblock *blk) { - return le16_to_cpu(blk->_len); + return le16_to_cpu(blk->len); } static inline u32 pdr_id(const struct pdr *pdr) { - return le32_to_cpu(pdr->_id); + return le32_to_cpu(pdr->id); } static inline u32 pdr_addr(const struct pdr *pdr) { - return le32_to_cpu(pdr->_addr); + return le32_to_cpu(pdr->addr); } static inline u32 pdr_len(const struct pdr *pdr) { - return le32_to_cpu(pdr->_len); + return le32_to_cpu(pdr->len); } static inline u32 pdi_id(const struct pdi *pdi) { - return le16_to_cpu(pdi->_id); + return le16_to_cpu(pdi->id); } /* Return length of the data only, in bytes */ static inline u32 pdi_len(const struct pdi *pdi) { - return 2 * (le16_to_cpu(pdi->_len) - 1); + return 2 * (le16_to_cpu(pdi->len) - 1); } - 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
[PATCHES] Bringing Orinoco to 0.15
Hello! I'm about to submit series of 21 patches for Orinoco drivers. Those patches have been in Orinoco CVS for months, and there have been no bug reports. I believe it's about time that I submit the patches to the kernel. I know, I've missed the boat for 2.6.17 (sorry for that, I was really overworked), but I hope it's better to submit the patches as early as possible so that they get some public exposure before ending up in the kernel. The changes are minimal, despite the size of the patches. The intention was to make as much cleanup as possible and call it 0.15, and then start doing real changes. The patches are available at http://red-bean.com/proski/orinoco/ This includes the cumulative patch and a zip archive with all patches. Andrew, maybe you you include the patches into the "mm" kernel? Here's a brief overview of the changes: 1) Remove old cruft, such as obsolete PCMCIA code, obsolete comments, obsolete debug code. 2) Fix misleading comments. For instance, Symbol Spectrum24 LA4137 was called Symbol Spectrum24 LA4100. 3) Fixes for minor annoyances, such as a kernel warning emitted every time when iwconfig requests stats on recent Symbol firmware. 4) Optimization for Tx exception handling. No need to read the complete Tx descriptor if just a few fields are needed. 5) Serious rework of the Tx code. The low level function for copying data to the card has been changed to accept odd number of bytes, which allows to remove all padding code used in the kernel. Not a single unwanted byte would leak to the air. Also, error handling has been redone, transmit errors are counted in one place, and orinoco_xmit() returns what the network layer expects it to, rather than -errno. 802.3 encapsulation has been slightly simplified. 6) The driver will reset the card in case of some bad Tx errors. 7) The driver will print the firmware version before trying to allocate the transmission buffer, so that users can report the firmware version in case of errors. 8) "Pseudo-PCI" drivers (i.e. drivers for cards in primitive PCI-to-PCMCIA adaptors) support suspend and resume now. 9) PCI suspend frees the IRQ and disables the device. 10) All I/O is done using the iomem API. 11) The code common between PCI and pseudo-PCI drivers has been put to orinoco_pci.h. The drivers have been changed to minimize differences between them. Sorry, it's a big patch, but that's how it was made, and splitting it would be rewriting history. I did split what I could (comment changes). -- Regards, Pavel Roskin - 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: Broadcast ARP packets on link local addresses
On Fri, 31 Mar 2006 15:26:00 -0800, David Daney wrote: > From: David Daney > > Greetings, > > When an internet host joins a network where there is no DHCP server, > it may auto-allocate an IP address by the method described in RFC > 3927. There are several user space daemons available that implement > most of the protocol (zcip, busybox, ...). The kernel's APR driver > should function in the normal manner except that it is required to > broadcast all ARP packets that it originates in the link local address > space (169.254.0.0/16). RFC 3927 section 2.5 explains the requirement. > > The current ARP code is non-compliant because it does not broadcast > some ARP packets as required by RFC 3927. I haven't seem anyone comment on this, but it would be useful to see this integrated. If you have a bunch of Linux machines and two machines have the same address allocated (not so impossible) they won't notice until they start communicating with each other. A third machine attempting to communicate with either party won't be able to do so. With this patch in place, overlapping assignments are noticed much faster due to the broadcast. Something else I've noticed while I've been implementing my zeroconf daemon is that the kernel returns link-scoped primary addresses first to 'ifconfig'. Unfortunately quite a lot of user-space programs parse its output and interpret the address it presents as the primary for the specified interface. Is that a case of user-space breakage that the kernel team would ordinarily worry about? Thanks, Anand - 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