Re: BUG: soft lockup detected on CPU#0! (2.6.18.2 plus hacks)

2007-01-11 Thread Jarek Poplawski
On Wed, Jan 10, 2007 at 11:40:35PM -0800, David Miller wrote:
 From: Jarek Poplawski [EMAIL PROTECTED]
 Date: Thu, 11 Jan 2007 08:24:28 +0100
 
  Yesterday I did what I should do earlier - checked
  this simple way, with printk, and now I have no doubts
  it's a bug: if you add or remove vlan devices with
  vconfig, register_vlan_device and unregister_vlan_dev
  are called by ioctl and they use and change rcu
  procetded data without preemption disabled so vlan
  rcu hash lists could become corrupted or find results
  could be wrong.
 
 Those two operations do their modifications and changes under the RTNL
 semaphore, via rtnl_lock() and rtnl_unlock() which guarentees that no
 other modifications can occur.

Sure, but is this even legal to be preempted during
reading or modifying rcu list? Doesn't this disturb
rcu cycle and make possible memory release problems?

Jarek P.
-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: BUG: soft lockup detected on CPU#0! (2.6.18.2 plus hacks)

2007-01-11 Thread Jarek Poplawski
On Thu, Jan 11, 2007 at 09:29:58AM +0100, Jarek Poplawski wrote:
 On Wed, Jan 10, 2007 at 11:40:35PM -0800, David Miller wrote:
  From: Jarek Poplawski [EMAIL PROTECTED]
  Date: Thu, 11 Jan 2007 08:24:28 +0100
  
   Yesterday I did what I should do earlier - checked
   this simple way, with printk, and now I have no doubts
   it's a bug: if you add or remove vlan devices with
   vconfig, register_vlan_device and unregister_vlan_dev
   are called by ioctl and they use and change rcu
   procetded data without preemption disabled so vlan
   rcu hash lists could become corrupted or find results
   could be wrong.
  
  Those two operations do their modifications and changes under the RTNL
  semaphore, via rtnl_lock() and rtnl_unlock() which guarentees that no
  other modifications can occur.
 
 Sure, but is this even legal to be preempted during

I should even say:

... is this even legal to be blocked during ...

 reading or modifying rcu list? Doesn't this disturb
 rcu cycle and make possible memory release problems?

Jarek P.
-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: BUG: soft lockup detected on CPU#0! (2.6.18.2 plus hacks)

2007-01-11 Thread Jarek Poplawski
On Thu, Jan 11, 2007 at 09:35:26AM +0100, Jarek Poplawski wrote:
 On Thu, Jan 11, 2007 at 09:29:58AM +0100, Jarek Poplawski wrote:
  On Wed, Jan 10, 2007 at 11:40:35PM -0800, David Miller wrote:
   From: Jarek Poplawski [EMAIL PROTECTED]
   Date: Thu, 11 Jan 2007 08:24:28 +0100
   
Yesterday I did what I should do earlier - checked
this simple way, with printk, and now I have no doubts
it's a bug: if you add or remove vlan devices with
vconfig, register_vlan_device and unregister_vlan_dev
are called by ioctl and they use and change rcu
procetded data without preemption disabled so vlan
rcu hash lists could become corrupted or find results
could be wrong.
   
   Those two operations do their modifications and changes under the RTNL
   semaphore, via rtnl_lock() and rtnl_unlock() which guarentees that no
   other modifications can occur.
  
  Sure, but is this even legal to be preempted during
 
 I should even say:
 
 ... is this even legal to be blocked during ...
 
  reading or modifying rcu list? Doesn't this disturb
  rcu cycle and make possible memory release problems?

Sorry, one more time:

Sure, but is this even legal to be preempted during
reading or modifying rcu list or be blocked while 
holding rcu protected pointer? Doesn't this disturb
rcu cycle and make possible memory release problems?

Jarek P.
-
To unsubscribe from this list: send 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: Fw: Intermittent SCTP multihoming breakage

2007-01-11 Thread Steve Hill
On Wed, 10 Jan 2007, Sridhar Samudrala wrote:

 So looks like there may be an issue with PR-SCTP(partial reliability)
 support and packet loss. I will take a look into this.

 Do you still see this problem even if you don't set timetolive?

No, the problem seems to go away if the timetolive is set to 0, so this is
what I have now done since I had not intended to set the timetolive in the
first place (but I thought it was still worth posting details of the
problem since it does appear to be a bug).

-- 
 - Steve Hill
   Software Engineer
   Dialogic
   Fordingbridge, Hampshire, UK
   +44-1425-651392
   [EMAIL PROTECTED]
-
To unsubscribe from this list: send 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] Re: kernel BUG in eth_alloc_tx_desc_index at drivers/net/mv643xx_eth.c:1069!

2007-01-11 Thread Jarek Poplawski
On Wed, Jan 10, 2007 at 06:12:29PM +0100, Thibaut VARENE wrote:
 On 1/9/07, Thibaut VARENE [EMAIL PROTECTED] wrote:
 On 1/9/07, Dale Farnsworth [EMAIL PROTECTED] wrote:
 
  Thank you Thibaut.  Please try the following patch:
 
  From: Dale Farnsworth [EMAIL PROTECTED]
 
  Reserve one unused descriptor in the TX ring
  to facilitate testing for when the ring is full.
 
 Dale,
 
 tried it and unfortunately:
 
 Also, I don't know if you read that bit, but everytime I reboot the
 box immediately after a crash, the NIC gets a bogus (always the same
 it seems) MAC address, and I have to reboot one more time to get back
 to the normal MAC address.
 
 Dunno if that hints anything though.

There is something in the code about MAC writing and saving
some config during initialization, so probably it's possible
if reinitialization was broken.

I tried to look more into the code and here are my (maybe
wrong) conclusions:

- It looks like something could be broken during tx descs
freeing or eth_tx_timeout_task. I compared the timeout code
with e100 and tg3 and have a feeling mv643xx_eth is doing 
less but I'm not able to estimate the importance of this.

- Such errors, IMHO, could be possible with races and not
enough locking, and btw. I think suspected function isn't
properly locked: mp-tx_desc_count in while condition isn't
protected at all. Below I attach a patch proposal but I'm
not sure some irq off or spin_lock isn't also needed
elswere. If it's only locking it would be suitable to do
the test with a kernel compiled without PREEMPT and SMP,
but if irqs nothing should change... 

Regards,
Jarek P.

PS: alas I didn't even check compiling - I had no time to
find all compile dependencies of this driver
---

Signed-off-by: Jarek Poplawski [EMAIL PROTECTED]
---

diff -Nurp linux-2.6.20-rc4-/drivers/net/mv643xx_eth.c 
linux-2.6.20-rc4/drivers/net/mv643xx_eth.c
--- linux-2.6.20-rc4-/drivers/net/mv643xx_eth.c 2006-12-18 08:57:52.0 
+0100
+++ linux-2.6.20-rc4/drivers/net/mv643xx_eth.c  2007-01-11 08:55:34.0 
+0100
@@ -312,8 +312,8 @@ int mv643xx_eth_free_tx_descs(struct net
int count;
int released = 0;
 
+   spin_lock_irqsave(mp-lock, flags);
while (mp-tx_desc_count  0) {
-   spin_lock_irqsave(mp-lock, flags);
tx_index = mp-tx_used_desc_q;
desc = mp-p_tx_desc_area[tx_index];
cmd_sts = desc-cmd_sts;
@@ -348,8 +348,10 @@ int mv643xx_eth_free_tx_descs(struct net
dev_kfree_skb_irq(skb);
 
released = 1;
+   spin_lock_irqsave(mp-lock, flags);
}
 
+   spin_unlock_irqrestore(mp-lock, flags);
return released;
 }
 
-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: BUG: soft lockup detected on CPU#0! (2.6.18.2 plus hacks)

2007-01-11 Thread Jarek Poplawski
On Thu, Jan 11, 2007 at 01:27:55AM -0800, David Miller wrote:
 From: Jarek Poplawski [EMAIL PROTECTED]
 Date: Thu, 11 Jan 2007 09:39:34 +0100
 
  Sure, but is this even legal to be preempted during
  reading or modifying rcu list or be blocked while 
  holding rcu protected pointer? Doesn't this disturb
  rcu cycle and make possible memory release problems?
 
 It's fine in this case.
 
 Since the list cannot be changed by anyone else, and the hash linked
 list (as seen by readers) is modified atomically by a single store, it
 all works out.
 
 Readers only look at foo-next in the hash traversal.  Since the
 preceeding element cannot change outside of the current writer,
 the -next pointer to update is protected.
 
 Readers therefore will either see the hash list with the entry or
 without.
 
 We then use call_rcu() to make sure any reading threads that happened
 to get a glimpse of the hash entry before the hlist_del_rcu()
 completed will go away and drop their references before we free that
 entry.
 
 I really don't see any problem here. :-)

Probably because you care more about internals and less 
about docs examples. It seems I'm too much about regulations. 
 
OK, I take your word and will try to stop annoy this list
with imagined RCU bugs, sorry.

Thanks for your precious (sleeping?) time
and explanations. Best regards,

Jarek P.
-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [IPSEC] flow: Cache negative results

2007-01-11 Thread Herbert Xu
On Wed, Jan 10, 2007 at 10:06:05PM -0800, David Miller wrote:
 
 One thing we could do is force a full flow cache flush when some
 percentage before the generation count would overflow.  Say every
 %80 of the generation ID space?  That gives enough space to ensure
 the flush tasklet runs before the ID really does wrap.

Yes that would be a good solution.  It'd prevent overflows from causing
any damage and is also pretty light weight (just a single comparison
every time the genid changes).

Cheers,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmVHI~} [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: IrDA spams logfiles - since 2.6.19

2007-01-11 Thread Samuel Ortiz
Hi Dave,

On Wed, Jan 10, 2007 at 03:26:07PM -0800, David Miller wrote:
 The warning is a bit extreme because the skb_cow() call does fix
 things up and makes space available.
 
 But it's not the most efficient thing in the world, so it's good
 that we know about this so we can have a closer work.
 
 I want to remove that warning, but first let's figure out what the
 call chain is that causes skb_headroom() to end up being zero.
In Andreas case, the device is probably in discovery mode, sending IrDA
discovery frames periodically. For each of them, the path is the following:
irlap_slot_timer_expired()
  irlap_do_event()
irlap_state_query()
  irlap_send_discovery_xid_frame()
dev_hard_start_xmit()
  irda_usb_hard_xmit()

irlap_send_discovery_xid_frame() is the one allocating and building the
discovery frame. It is an IrLAP frame.
As you might remember, this routine used to call dev_alloc_skb() for 
allocation but now calls alloc_skb(). So, it used to reserve a 16 bytes
header, while now it doesn't.
The issue here is that the USB-IrDA specification tells us that we have to add
a 1 byte header to the IrLAP frame (and this becomes 3 bytes for some devices
playing with the spec) before pushing it to the USB bus.
So, while the discovery routine returns a correct IrLAP skb, with no headroom,
the irda-usb code needs to add a 1 (or 3) bytes header to it.

One solution to fix that problem could be to systematically add a header for
every allocated IrLAP frame on the IrDA TX path. Since the USB-IrDA spec is
defined by the USB-IF, not by the IrDA, I don't really like this solution. It
would create code changes all over the IrDA stack only for handling the
USB-IrDA case.
The second solution would consist of copying the skb-data into some irda-usb.c
pre-allocated buffer, instead of always calling skb_cow(). That would save us
one kmalloc(). Obviously, there is a performance hit as we're doing one memcpy
for each and every packet sent. I think this is a cleaner solution but the
performance hit must be taken to account since the IrDA USB dongles are among
the most popular IrDA devices.

Dave, please let me know what you think about it, and if you agree with my
problem description, please let me know which solution you would advice for.

The patch for the second solution would look something like that (although
I could use some of the skb routines - skb_copy_bits() - to copy the skb
data). It's been tested with my USB stir4220 based IrDA dongle:

diff --git a/drivers/net/irda/irda-usb.h b/drivers/net/irda/irda-usb.h
index 6b2271f..e846c38 100644
--- a/drivers/net/irda/irda-usb.h
+++ b/drivers/net/irda/irda-usb.h
@@ -156,6 +156,7 @@ struct irda_usb_cb {
struct irlap_cb   *irlap;   /* The link layer we are binded to */
struct qos_info qos;
char *speed_buff;   /* Buffer for speed changes */
+   char *tx_buff;
 
struct timeval stamp;
struct timeval now;
diff --git a/drivers/net/irda/irda-usb.c b/drivers/net/irda/irda-usb.c
index 3ca1082..5f22a8e 100644
--- a/drivers/net/irda/irda-usb.c
+++ b/drivers/net/irda/irda-usb.c
@@ -441,25 +441,14 @@ static int irda_usb_hard_xmit(struct sk_buff *skb, struct 
net_device *netdev)
goto drop;
}
 
-   /* Make sure there is room for IrDA-USB header. The actual
-* allocation will be done lower in skb_push().
-* Also, we don't use directly skb_cow(), because it require
-* headroom = 16, which force unnecessary copies - Jean II */
-   if (skb_headroom(skb)  self-header_length) {
-   IRDA_DEBUG(0, %s(), Insuficient skb headroom.\n, 
__FUNCTION__);
-   if (skb_cow(skb, self-header_length)) {
-   IRDA_WARNING(%s(), failed skb_cow() !!!\n, 
__FUNCTION__);
-   goto drop;
-   }
-   }
+   memset(self-tx_buff, 0, IRDA_SKB_MAX_MTU + self-header_length);
+   memcpy(self-tx_buff + self-header_length, skb-data, skb-len);
 
/* Change setting for next frame */
-
if (self-capability  IUC_STIR421X) {
__u8 turnaround_time;
-   __u8* frame;
+   __u8* frame = self-tx_buff;
turnaround_time = get_turnaround_time( skb );
-   frame= skb_push(skb, self-header_length);
irda_usb_build_header(self, frame, 0);
frame[2] = turnaround_time;
if ((skb-len != 0) 
@@ -472,17 +461,18 @@ static int irda_usb_hard_xmit(struct sk_buff *skb, struct 
net_device *netdev)
frame[1] = 0;
}
} else {
-   irda_usb_build_header(self, skb_push(skb, self-header_length), 
0);
+   irda_usb_build_header(self, self-tx_buff, 0);
}
 
/* FIXME: Make macro out of this one */
((struct irda_skb_cb *)skb-cb)-context = self;
 
-usb_fill_bulk_urb(urb, self-usbdev, 
+   usb_fill_bulk_urb(urb, 

Re: fixing opt-ack DoS against TCP stack

2007-01-11 Thread Gavin McCullagh
Hi,

[ moving this to netdev as requested ]

On Tue, 09 Jan 2007, Stephen Hemminger wrote:

 Actually, this paper seems to be a zombified version of:
   http://www.cs.ucsd.edu/~savage/papers/CCR99.pdf

Thanks.  In fairness to them, the emphasis is slightly different, Savage et
al are more interested in improving a receiver's performance, whereas
Sherwood seems more interested in a DoS attack.  However...

 It is not clear that current Linux systems are prone to the attack for a
 couple of reasons. First, Linux does more counts packets not bytes so
 extra ack's would be ignored. Turning on ABC would also help.

The issue I'm raising is not as much how you could artifically increase
Cwnd by dividing ACKs or sending them early.  The issue as I see it is that
if the receiver doesn't send dup acks, the sender never backs off and may
eventually flood its own link.  As the receiver need only ACK the odd
packet, the amplification can be substantial.

As this issue is already moreorless solved (in the research sense), I'm not
keen to spend a lot of time writing it up.  So, here's a quick throw
together of a small example experiment I ran yesterday.

http://www.hamilton.ie/gavinmc/drop_dupack_attack/

 Lastly, the patch looks like it could cause more problems. It probably would
 break some application and other non-attacking TCP stacks. For this case, IMHO
 we need to wait for more research. If you want to pursue the problem, it needs
 to go through the RFC process.

I must admit I didn't read the patch in detail.  As I understand it, the
fix should (in principal) be compatible with other TCP stacks who should
just see an odd extra dropped packet and react with a duplicate ack as
usual.

Gavin

-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH/RFC 01/10] Implement local diversion of IPv4 skbs

2007-01-11 Thread KOVACS Krisztian

  Hi,

On Wednesday 10 January 2007 13:32, Patrick McHardy wrote:
 How exactly are dynamic ports handled? Do you just add a catch-all rule
 that filters based on socket lookups?

 In that case you could do something like this:

 ip route add local default dev lo scope host table 1
 ip rule add fwmark 0x1 lookup 1

 and still use the socket lookups for marking, which would (without the
 socket caching) remove the need for this patch entirely.

  Ok, I'll try to address all the concerns raised on the list.

  Thanks a lot for the review and comments.

-- 
 Regards,
  Krisztian Kovacs
-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


RCU info

2007-01-11 Thread Stephen Hemminger
Paul McKenney has given lots of talks on RCU see:
http://www.rdrop.com/users/paulmck/RCU/

and look in Documentation/RCU
-
To unsubscribe from this list: send 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 3/4] [SCTP]: Correctly handle unexpected INIT-ACK chunk.

2007-01-11 Thread Sridhar Samudrala
[SCTP]: Correctly handle unexpected INIT-ACK chunk.

Consider the chunk as Out-of-the-Blue if we don't have
an endpoint.  Otherwise discard it as before.

Signed-off-by: Vlad Yasevich [EMAIL PROTECTED]
Signed-off-by: Sridhar Samudrala [EMAIL PROTECTED]

---
 include/net/sctp/sm.h|1 +
 net/sctp/sm_statefuns.c  |   22 ++
 net/sctp/sm_statetable.c |2 +-
 3 files changed, 24 insertions(+), 1 deletions(-)

diff --git a/include/net/sctp/sm.h b/include/net/sctp/sm.h
index 3269ed1..73cb994 100644
--- a/include/net/sctp/sm.h
+++ b/include/net/sctp/sm.h
@@ -134,6 +134,7 @@ sctp_state_fn_t sctp_sf_violation;
 sctp_state_fn_t sctp_sf_discard_chunk;
 sctp_state_fn_t sctp_sf_do_5_2_1_siminit;
 sctp_state_fn_t sctp_sf_do_5_2_2_dupinit;
+sctp_state_fn_t sctp_sf_do_5_2_3_initack;
 sctp_state_fn_t sctp_sf_do_5_2_4_dupcook;
 sctp_state_fn_t sctp_sf_unk_chunk;
 sctp_state_fn_t sctp_sf_do_8_5_1_E_sa;
diff --git a/net/sctp/sm_statefuns.c b/net/sctp/sm_statefuns.c
index 2c165dc..a10c3a8 100644
--- a/net/sctp/sm_statefuns.c
+++ b/net/sctp/sm_statefuns.c
@@ -1534,6 +1534,28 @@ sctp_disposition_t sctp_sf_do_5_2_2_dupinit(const struct 
sctp_endpoint *ep,
 }


+/*
+ * Unexpected INIT-ACK handler.
+ *
+ * Section 5.2.3
+ * If an INIT ACK received by an endpoint in any state other than the
+ * COOKIE-WAIT state, the endpoint should discard the INIT ACK chunk.
+ * An unexpected INIT ACK usually indicates the processing of an old or
+ * duplicated INIT chunk.
+*/
+sctp_disposition_t sctp_sf_do_5_2_3_initack(const struct sctp_endpoint *ep,
+   const struct sctp_association *asoc,
+   const sctp_subtype_t type,
+   void *arg, sctp_cmd_seq_t *commands)
+{
+   /* Per the above section, we'll discard the chunk if we have an
+* endpoint.  If this is an OOTB INIT-ACK, treat it as such.
+*/
+if (ep == sctp_sk((sctp_get_ctl_sock()))-ep)
+   return sctp_sf_ootb(ep, asoc, type, arg, commands);
+   else
+   return sctp_sf_discard_chunk(ep, asoc, type, arg, commands);
+}

 /* Unexpected COOKIE-ECHO handler for peer restart (Table 2, action 'A')
  *
diff --git a/net/sctp/sm_statetable.c b/net/sctp/sm_statetable.c
index 733dd87..5f6cc7a 100644
--- a/net/sctp/sm_statetable.c
+++ b/net/sctp/sm_statetable.c
@@ -152,7 +152,7 @@ const sctp_sm_table_entry_t 
*sctp_sm_lookup_event(sctp_event_t event_type,
/* SCTP_STATE_EMPTY */ \
TYPE_SCTP_FUNC(sctp_sf_ootb), \
/* SCTP_STATE_CLOSED */ \
-   TYPE_SCTP_FUNC(sctp_sf_discard_chunk), \
+   TYPE_SCTP_FUNC(sctp_sf_do_5_2_3_initack), \
/* SCTP_STATE_COOKIE_WAIT */ \
TYPE_SCTP_FUNC(sctp_sf_do_5_1C_ack), \
/* SCTP_STATE_COOKIE_ECHOED */ \
--

-
To unsubscribe from this list: send 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 1/4] [SCTP]: Set correct error cause value for missing parameters

2007-01-11 Thread Sridhar Samudrala
Hi Dave,

Here is a set of 4 SCTP patches that fix minor protocol 
conformance issues found by Vlad when running a conformance
test suite.
Please consider them for inclusion in 2.6.20.

Thanks
Sridhar

[SCTP]: Set correct error cause value for missing parameters

sctp_process_missing_param() needs to use the SCTP_ERROR_MISS_PARAM
error cause value.

Signed-off-by: Vlad Yasevich [EMAIL PROTECTED]
Signed-off-by: Sridhar Samudrala [EMAIL PROTECTED]

---
 net/sctp/sm_make_chunk.c |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/net/sctp/sm_make_chunk.c b/net/sctp/sm_make_chunk.c
index 167d888..ea0f8fa 100644
--- a/net/sctp/sm_make_chunk.c
+++ b/net/sctp/sm_make_chunk.c
@@ -1562,7 +1562,7 @@ static int sctp_process_missing_param(const struct 
sctp_association *asoc,
if (*errp) {
report.num_missing = htonl(1);
report.type = paramtype;
-   sctp_init_cause(*errp, SCTP_ERROR_INV_PARAM,
+   sctp_init_cause(*errp, SCTP_ERROR_MISS_PARAM,
report, sizeof(report));
}

--

-
To unsubscribe from this list: send 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 2/4] [SCTP]: Verify some mandatory parameters.

2007-01-11 Thread Sridhar Samudrala
[SCTP]: Verify some mandatory parameters.

Verify init_tag and a_rwnd mandatory parameters in INIT and
INIT-ACK chunks.

Signed-off-by: Vlad Yasevich [EMAIL PROTECTED]
Signed-off-by: Sridhar Samudrala [EMAIL PROTECTED]

---
 net/sctp/sm_make_chunk.c |4 +++-
 net/sctp/sm_statefuns.c  |   19 ---
 2 files changed, 3 insertions(+), 20 deletions(-)

diff --git a/net/sctp/sm_make_chunk.c b/net/sctp/sm_make_chunk.c
index ea0f8fa..12c0e17 100644
--- a/net/sctp/sm_make_chunk.c
+++ b/net/sctp/sm_make_chunk.c
@@ -1775,7 +1775,9 @@ int sctp_verify_init(const struct sctp_association *asoc,

/* Verify stream values are non-zero. */
if ((0 == peer_init-init_hdr.num_outbound_streams) ||
-   (0 == peer_init-init_hdr.num_inbound_streams)) {
+   (0 == peer_init-init_hdr.num_inbound_streams) ||
+   (0 == peer_init-init_hdr.init_tag) ||
+   (SCTP_DEFAULT_MINWINDOW  ntohl(peer_init-init_hdr.a_rwnd))) {

sctp_process_inv_mandatory(asoc, chunk, errp);
return 0;
diff --git a/net/sctp/sm_statefuns.c b/net/sctp/sm_statefuns.c
index aa51d19..2c165dc 100644
--- a/net/sctp/sm_statefuns.c
+++ b/net/sctp/sm_statefuns.c
@@ -440,7 +440,6 @@ sctp_disposition_t sctp_sf_do_5_1C_ack(const struct 
sctp_endpoint *ep,
 {
struct sctp_chunk *chunk = arg;
sctp_init_chunk_t *initchunk;
-   __u32 init_tag;
struct sctp_chunk *err_chunk;
struct sctp_packet *packet;
sctp_error_t error;
@@ -462,24 +461,6 @@ sctp_disposition_t sctp_sf_do_5_1C_ack(const struct 
sctp_endpoint *ep,
/* Grab the INIT header.  */
chunk-subh.init_hdr = (sctp_inithdr_t *) chunk-skb-data;

-   init_tag = ntohl(chunk-subh.init_hdr-init_tag);
-
-   /* Verification Tag: 3.3.3
-*   If the value of the Initiate Tag in a received INIT ACK
-*   chunk is found to be 0, the receiver MUST treat it as an
-*   error and close the association by transmitting an ABORT.
-*/
-   if (!init_tag) {
-   struct sctp_chunk *reply = sctp_make_abort(asoc, chunk, 0);
-   if (!reply)
-   goto nomem;
-
-   sctp_add_cmd_sf(commands, SCTP_CMD_REPLY, SCTP_CHUNK(reply));
-   return sctp_stop_t1_and_abort(commands, SCTP_ERROR_INV_PARAM,
- ECONNREFUSED, asoc,
- chunk-transport);
-   }
-
/* Verify the INIT chunk before processing it. */
err_chunk = NULL;
if (!sctp_verify_init(asoc, chunk-chunk_hdr-type,
--

-
To unsubscribe from this list: send 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 4/4] [SCTP]: Fix SACK sequence during shutdown

2007-01-11 Thread Sridhar Samudrala
[SCTP]: Fix SACK sequence during shutdown

Currently, when association enters SHUTDOWN state,the
implementation will SACK any DATA first and then transmit
the SHUTDOWN chunk.  This is against the order required by
2960bis spec.  SHUTDOWN must always be first, followed by
SACK. This change forces this order and also enables bundling.

Signed-off-by: Vlad Yasevich [EMAIL PROTECTED]
Signed-off-by: Sridhar Samudrala [EMAIL PROTECTED]

---
 net/sctp/sm_sideeffect.c |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/net/sctp/sm_sideeffect.c b/net/sctp/sm_sideeffect.c
index 7bbc615..8bd3097 100644
--- a/net/sctp/sm_sideeffect.c
+++ b/net/sctp/sm_sideeffect.c
@@ -217,7 +217,7 @@ static int sctp_gen_sack(struct sctp_association *asoc, int 
force,

asoc-peer.sack_needed = 0;

-   error = sctp_outq_tail(asoc-outqueue, sack);
+   sctp_add_cmd_sf(commands, SCTP_CMD_REPLY, SCTP_CHUNK(sack));

/* Stop the SACK timer.  */
sctp_add_cmd_sf(commands, SCTP_CMD_TIMER_STOP,
--

-
To unsubscribe from this list: send 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] ethtool: fix long statistics name

2007-01-11 Thread Stephen Hemminger
Fix handling of statistics where the label is exactly 32 (ETH_GSTRING_LEN)
characters long (observed with chelsio 10G driver). 
Before it would print garbage because of going by end
of string. Don't need to copy string, just use formats properly.

Signed-off-by: Stephen Hemminger [EMAIL PROTECTED]
---
 ethtool.c |   10 --
 1 files changed, 4 insertions(+), 6 deletions(-)

diff --git a/ethtool.c b/ethtool.c
index 6e68009..0a50144 100644
--- a/ethtool.c
+++ b/ethtool.c
@@ -1989,12 +1989,10 @@ static int do_gstats(int fd, struct ifre
/* todo - pretty-print the strings per-driver */
fprintf(stdout, NIC statistics:\n);
for (i = 0; i  n_stats; i++) {
-   char s[ETH_GSTRING_LEN];
-
-   strncpy(s, (const char *) strings-data[i * ETH_GSTRING_LEN],
-   ETH_GSTRING_LEN);
-   fprintf(stdout,  %s: %llu\n,
-   s, stats-data[i]);
+   fprintf(stdout,  %.*s: %llu\n,
+   ETH_GSTRING_LEN, 
+   strings-data[i * ETH_GSTRING_LEN],
+   stats-data[i]);
}
free(strings);
free(stats);
-- 
1.4.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


Re: [openib-general] [RFC] [PATCH V2 0/3] bonding support for operation over IPoIB

2007-01-11 Thread Or Gerlitz

Or Gerlitz wrote:

This patch series is a second version (see below link to V1) of the suggested
changes to the bonding driver such that it would be able to support non 
ARPHRD_ETHER
netdevices for its High-Availability (active-backup) mode.

The motivation is to enable the bonding driver on its HA mode to work with the
IP over Infiniband (IPoIB) driver. With these patches I was able to enslave
IPoIB netdevices and run TCP, UDP, IP (UDP) Multicast and ICMP traffic with
fail-over and fail-back working fine. My working env was the net-2.6.20 git.




These patches are not enough for configuration of IPoIB bonding through tools
(eg /sbin/ifenslave and /sbin/ifup) provided by packages such as sysconfig and
initscripts, specifically since these tools sets the bonding device to be UP
before enslaving anything. Once this patchset gets positive/feedback the next 
step
would be to look how to enhance the tools/packages so it would be possible to
bond/enslave with the modified code. As suggested by the bonding maintainer, 
this
step can potentially involve converting ifenslave to be a script based on the 
bonding
sysfs infrastructure rather on the somehow obsoleted 
Documentation/networking/ifenslave.c


Jay,

I would like to move forward and push the V2 patch series upstream 
through netdev and then start working on the configuration tools etc 
changes needed to support bonding IPoIB devices through non direct 
bonding sysfs scripts... are you OK with that?


If you agree to the push, who is doing this nowadays, is it Jeff Garzik 
or David Miller?


Roland - any other comments/concerns that you might have are very much 
appreciated.


Or.

-
To unsubscribe from this list: send 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: [2.4 PATCH] ethernet (net/ethernet/eth.c): eth_header() may produce invalid packets (with dest. addr. = 00:00:00:00:00:00)

2007-01-11 Thread Willy Tarreau
On Wed, Jan 10, 2007 at 12:53:31PM +0100, Christian Praehauser wrote:
 Hello, and sorry for bothering you with a patch you've already seen ;-).

I had not seen it yet, so I'm fine with it :-)

 From: Christian Praehauser, Department of Computer Sciences, University of 
 Salzburg
 
 This patch fixes a problem which has already been corrected in Linux-2.6.16 
 but was not back-ported to the 2.4 series. It is essentially the same as 
 the patch for 2.6.16. An excerpt from the ChangeLog for Linux 2.6.16 is 
 included below.

David, I think that Christian and Alexey's descriptions are fine, and that
this patch should be merged. Are you OK ?

 What is not described in the patch description for 2.6.16 is that this 
 problem also arises when transmitting IP multicast packets. If you send an 
 IP multicast stream over an ethernet network interface ethX and turn off 
 ARP on ethX then Linux will produce an ethernet frame with a dest. 
 addresses of 00:00:00:00:00:00 (which is invalid). As IP multicast 
 addresses are directly mapped to HW (MAC) addresses without invoking any 
 ARP protocol mechanisms - for IP4 this mapping is performed by the function 
 ip_eth_mc_map - it makes perfect sense to do this even if ARP is disabled. 
 Further, this problem may occur periodically, everytime the corresponding 
 struct dst_entry is garbage-collected (e.g. ~ every 10 minutes).

Christian, would you please produce a properly formated description starting
from the 2.6 one and appending your comments to it ? I'd also like to get
the same subject as for the 2.6 patch. It's important that we keep patches
subjects and descriptions for backports, it helps further patch tracking.

Thanks in advance,
Willy

-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


What is in bcm43xx-wireless-dev.git?

2007-01-11 Thread Michael Buesch
In case you wonder what's the future of bcm43xx, here's
a list of changes in my bcm43xx development tree:

  bcm43xx-d80211: Add some PHY register definitions.
  bcm43xx-d80211: Move ILT stuff to OFDM table stuff
  bcm43xx-d80211: Remove PHY OFDM routing bit, if we are on A-PHY.
  bcm43xx-d80211: Merge new LO-control code.
  bcm43xx-d80211: Fix compilation: Missing files for LO and VSTACK.
  bcm43xx-d80211: Rename struct bcm43xx_phyinfo to struct bcm43xx_phy
  bcm43xx-d80211: merge struct bcm43xx_radioinfo into struct bcm43xx_phy
  bcm43xx-d80211: Merge all radio stuff into phy.c
  bcm43xx-d80211: Fix antenna selection for TX and RX.
  bcm43xx-d80211: Fix bogus LO validation failure.
  bcm43xx-d80211: Remove netpoll and ethtool stuff.
  Remove obsolete SSB driver library.
  Implement new SSB subsystem.
  bcm43xx-d80211: Port driver to the new SSB subsystem.

Well, that doesn't tell you anything, right?
Let's explain it:

Most significant changes are the new LO code and
the completely rewritten SSB subsystem.

The LO calibration code is not yet finished and contains
a few bugs, so it works _worse_ that the LO calibration
code that's in mainline kernel. But it's the first step
in the direction to support hwpctl cards (4318).

The other major change is the new SSB subsystem. This is
a big step in the embedded direction. The new SSB subsystem
makes it possible to run an (almost) vanilla (vanilla, as in
my tree, which is supposed to get merged upstream some time :))
kernel on broadcom MIPS based embedded WLAN routers (openWRT).

bcm43xx works on the new subsystem. A working b44 port is
available, but not included here. We might probably want to
merge that through jeff directly once this is upstream.
The SSB subsystem is able to boot my Linksys WRT54G.
Wireless and LAN with SSB based chips basically works.

So, what to do?
Next think will be to fix the LO code to make it ready
for upstream merge. I don't know if that means making
4318 usable, too. Hopefully it does. Let's see. ;)

If you want to test this, get it from my repository.
If you have a linville-wireless-dev tree around, please
_don't_ clone my tree, but simply pull my stuff into
a seperate branch of that tree:

git branch crazy_stuff
git checkout crazy_stuff
git pull http://bu3sch.de/git/wireless-dev.git master

That will save lots of bandwidth. (Yours and mine) Thanks! ;)

Here's also a bzipped patch between today's linville-wireless-dev
and my tree. Yeah, it's huge, so not attached. :)

http://bu3sch.de/misc/linville_to_buesch_20070111.patch.bz2

-- 
Greetings Michael.
-
To unsubscribe from this list: send 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] bcm43xx: Fix failure to deliver PCI-E interrupts

2007-01-11 Thread Larry Finger
The PCI-E modifications to bcm43xx do not set up the interrupt vector
correctly.

Signed-off-by: Larry Finger [EMAIL PROTECTED]
---

John,

This fix should be applied to wireless-2.6 _AND_ pushed upstream to
2.6.20-rcX. Without this patch, none of the PCI-E interfaces will work.

Larry


Index: linux-2.6/drivers/net/wireless/bcm43xx/bcm43xx_main.c
===
--- linux-2.6.orig/drivers/net/wireless/bcm43xx/bcm43xx_main.c
+++ linux-2.6/drivers/net/wireless/bcm43xx/bcm43xx_main.c
@@ -2704,7 +2704,7 @@ static int bcm43xx_probe_cores(struct bc
sb_id_hi = bcm43xx_read32(bcm, BCM43xx_CIR_SB_ID_HI);
 
/* extract core_id, core_rev, core_vendor */
-   core_id = (sb_id_hi  0xFFF0)  4;
+   core_id = (sb_id_hi  0x8FF0)  4;
core_rev = (sb_id_hi  0xF);
core_vendor = (sb_id_hi  0x)  16;
 
@@ -2717,7 +2717,7 @@ static int bcm43xx_probe_cores(struct bc
case BCM43xx_COREID_PCIE:
core = bcm-core_pci;
if (core-available) {
-   printk(KERN_WARNING PFX Multiple PCI cores 
found.\n);
+   printk(KERN_WARNING PFX Multiple PCI/PCI-E 
cores found.\n);
continue;
}
break;
@@ -2872,11 +2872,14 @@ static int bcm43xx_wireless_core_init(st
u32 sbimconfiglow;
u8 limit;
 
-   if (bcm-core_pci.rev = 5  bcm-core_pci.id != BCM43xx_COREID_PCIE) {
+   if (bcm-core_pci.rev = 5  bcm-core_pci.id == BCM43xx_COREID_PCI) {
sbimconfiglow = bcm43xx_read32(bcm, BCM43xx_CIR_SBIMCONFIGLOW);
sbimconfiglow = ~ BCM43xx_SBIMCONFIGLOW_REQUEST_TOUT_MASK;
sbimconfiglow = ~ BCM43xx_SBIMCONFIGLOW_SERVICE_TOUT_MASK;
-   sbimconfiglow |= 0x32;
+   if (bcm-bustype == BCM43xx_BUSTYPE_PCI)
+   sbimconfiglow |= 0x32;
+   else
+   sbimconfiglow |= 0x53;
bcm43xx_write32(bcm, BCM43xx_CIR_SBIMCONFIGLOW, sbimconfiglow);
}
 
@@ -3070,6 +3073,7 @@ static int bcm43xx_setup_backplane_pci_c
u32 backplane_flag_nr;
u32 value;
struct bcm43xx_coreinfo *old_core;
+   struct bcm43xx_coreinfo *pci_core = bcm-core_pci ;
int err = 0;
 
value = bcm43xx_read32(bcm, BCM43xx_CIR_SBTPSFLAG);
@@ -3080,26 +3084,22 @@ static int bcm43xx_setup_backplane_pci_c
if (err)
goto out;
 
-   if (bcm-current_core-rev  6 ||
-   bcm-current_core-id == BCM43xx_COREID_PCI) {
-   value = bcm43xx_read32(bcm, BCM43xx_CIR_SBINTVEC);
-   value |= (1  backplane_flag_nr);
-   bcm43xx_write32(bcm, BCM43xx_CIR_SBINTVEC, value);
-   } else {
+   if ((pci_core-rev = 6) || (pci_core-id == BCM43xx_COREID_PCIE)) {
err = bcm43xx_pci_read_config32(bcm, BCM43xx_PCICFG_ICR, 
value);
-   if (err) {
-   printk(KERN_ERR PFX Error: ICR setup failure!\n);
+   if (err)
goto out_switch_back;
-   }
value |= core_mask  8;
err = bcm43xx_pci_write_config32(bcm, BCM43xx_PCICFG_ICR, 
value);
-   if (err) {
-   printk(KERN_ERR PFX Error: ICR setup failure!\n);
+   if (err)
goto out_switch_back;
-   }
+   } else {
+err = -EINVAL;
+value = bcm43xx_read32(bcm, BCM43xx_CIR_SBINTVEC);
+   value |= 1  backplane_flag_nr;
+bcm43xx_write32(bcm, BCM43xx_CIR_SBINTVEC, value);
}
 
-   if (bcm-current_core-id == BCM43xx_COREID_PCI) {
+   if (pci_core-id == BCM43xx_COREID_PCI) {
value = bcm43xx_read32(bcm, BCM43xx_PCICORE_SBTOPCI2);
value |= BCM43xx_SBTOPCI2_PREFETCH | BCM43xx_SBTOPCI2_BURST;
bcm43xx_write32(bcm, BCM43xx_PCICORE_SBTOPCI2, value);
@@ -3118,21 +3118,21 @@ static int bcm43xx_setup_backplane_pci_c
value |= BCM43xx_SBTOPCI2_MEMREAD_MULTI;
bcm43xx_write32(bcm, BCM43xx_PCICORE_SBTOPCI2, value);
}
-   } else {
-   if (bcm-current_core-rev == 0 || bcm-current_core-rev == 1) 
{
+   } else if (pci_core-id == BCM43xx_COREID_PCIE) {
+   if (pci_core-rev == 0 || pci_core-rev == 1) {
value = bcm43xx_pcie_reg_read(bcm, 
BCM43xx_PCIE_TLP_WORKAROUND);
value |= 0x8;
bcm43xx_pcie_reg_write(bcm, BCM43xx_PCIE_TLP_WORKAROUND,
   value);
}
-   if (bcm-current_core-rev == 0) {
+   if (pci_core-rev == 0) {

Re: What is in bcm43xx-wireless-dev.git?

2007-01-11 Thread Pavel Roskin
Hello, Michael!

I've tried the master branch of bcm43xx-wireless-dev.git for the first
time.  The results are not encouraging so far (although I'm quite new to
this chipset).

drivers/ssb/driver_mips/mips.c includes asm/time.h, which is missing on
x86_64.  It also refers to struct ssb_serial_ports, which is not defined
anywhere.

drivers/ssb/driver_pci/pcicore.c refers to SSB_PCICORE_SBTOPCI1_CFG1
that is not defined anywhere in the kernel.

I could send you huge logs, but I think it's already a clear sign that
you didn't compile-test the current tree with CONFIG_SSB_DRIVER_MIPS
enabled.  Things are better if it is disabled.

In case you haven't seen those warnings:

/home/proski/src/linux-2.6/drivers/net/wireless/d80211/bcm43xx/bcm43xx_main.c:843:
 warning: 'bcm43xx_wireless_core_mark_inactive' defined but not used
/home/proski/src/linux-2.6/drivers/net/wireless/d80211/bcm43xx/bcm43xx_main.c:3032:
 warning: 'bcm43xx_bluetooth_coext_enable' defined but not used

I could compile the code, and it even loaded as a module, but the kernel
oopsed once I brought wlan0 up by ifconfig.

bcm43xx_d80211: Adding Interface type 2
bcm43xx_d80211: Found PHY: Version 4, Type 2, Revision 8
bcm43xx_d80211: Found Radio: Manuf 0x17F, Version 0x2050, Revision 2
bcm43xx_d80211: Loading firmware version 351.126 (2006-07-29 05:54:02)
bcm43xx_d80211: Radio turned on
bcm43xx_d80211: FIXME: Possibly broken code in bcm43xx_phy_initg() at 
/home/proski/src/linux-2.6/drivers/net/wireless/d80211/bcm43xx/bcm43xx_phy.c:1650
bcm43xx_d80211: Chip initialized
Unable to handle kernel NULL pointer dereference at  RIP: 
 [8021eee6] dma_alloc_coherent+0x52/0x23f
PGD 50d1067 PUD 5201067 PMD 0 
Oops:  [1] SMP 
CPU 1 
Modules linked in: bcm43xx_d80211 ssb
Pid: 2623, comm: ifconfig Not tainted 2.6.20-rc3 #5
RIP: 0010:[8021eee6]  [8021eee6] 
dma_alloc_coherent+0x52/0x23f
RSP: 0018:810005127bd8  EFLAGS: 00010206
RAX:  RBX: 3500 RCX: 10d4
RDX:  RSI: 1000 RDI: 81000196c3a8
RBP: 10d0 R08: 81001f6d1600 R09: 0004
R10: 0001 R11: 810001122c60 R12: 1000
R13: 810001b96000 R14: 81000196c3a8 R15: 
FS:  2b411b2133b0() GS:810001827440() knlGS:
CS:  0010 DS:  ES:  CR0: 8005003b
CR2:  CR3: 15fdf000 CR4: 06e0
Process ifconfig (pid: 2623, threadinfo 810005126000, task 81001f58d180)
Stack:  81001ee6a3c0 81001ee6a3e8 810001b96000 3500
 81001ee6a3c0 0080 810001b96000 
  88019966 0005 00010036
Call Trace:
 [88019966] :bcm43xx_d80211:bcm43xx_setup_dmaring+0x1a7/0x460
 [88019ea3] :bcm43xx_d80211:bcm43xx_dma_init+0xc0/0x290
 [88008131] :bcm43xx_d80211:bcm43xx_wireless_core_init+0x78f/0x9f1
 [8022885e] mntput_no_expire+0x19/0x77
 [8020a3c3] do_page_fault+0x40b/0x74a
 [88009f4e] :bcm43xx_d80211:bcm43xx_add_interface+0x67/0xeb
 [804378d2] ieee80211_open+0x1c6/0x2c5
 [803fd3ec] dev_open+0x2f/0x6e
 [803fbcd5] dev_change_flags+0x5a/0x119
 [80423bab] devinet_ioctl+0x235/0x59c
 [803f4bef] sock_ioctl+0x1c8/0x1e5
 [8023b862] do_ioctl+0x21/0x6b
 [8022b614] vfs_ioctl+0x25c/0x275
 [8024559b] sys_ioctl+0x3c/0x5c
 [802548ee] system_call+0x7e/0x83


Code: 4c 23 38 49 39 d7 0f 46 e9 49 8d 44 24 ff 83 ca ff 49 89 c5 
RIP  [8021eee6] dma_alloc_coherent+0x52/0x23f
 RSP 810005127bd8
CR2: 

It's Fedora Core 6 x86_64 on a Dell Latitude with Core 2 Duo and an PCIe
wireless card.  SMP is enabled.  From .config:

CONFIG_BCM43XX_D80211=m
CONFIG_BCM43XX_D80211_PCI=y
CONFIG_BCM43XX_D80211_DEBUG=y
CONFIG_BCM43XX_D80211_DMA=y
CONFIG_BCM43XX_D80211_PIO=y
CONFIG_BCM43XX_D80211_DMA_AND_PIO_MODE=y
# CONFIG_BCM43XX_D80211_DMA_MODE is not set
# CONFIG_BCM43XX_D80211_PIO_MODE is not set
[skip]
CONFIG_SSB=m
# CONFIG_SSB_SILENT is not set
# CONFIG_SSB_DEBUG is not set
CONFIG_SSB_DRIVER_EXTIF=y
# CONFIG_SSB_DRIVER_MIPS is not set

[EMAIL PROTECTED] proski]# lspci -v -s 0c:00.0
0c:00.0 Network controller: Broadcom Corporation BCM4310 UART (rev 01)
Subsystem: Dell Unknown device 0007
Flags: bus master, fast devsel, latency 0, IRQ 17
Memory at efdfc000 (32-bit, non-prefetchable) [size=16K]
Capabilities: [40] Power Management version 2
Capabilities: [58] Message Signalled Interrupts: 64bit- Queue=0/0 
Enable-
Capabilities: [d0] Express Legacy Endpoint IRQ 0
Capabilities: [100] Advanced Error Reporting
Capabilities: [13c] Virtual Channel

[EMAIL PROTECTED] proski]# lspci -n -s 0c:00.0
0c:00.0 0280: 14e4:4312 (rev 01)
[EMAIL PROTECTED] proski]#

P.S.  Second attempt, this time with pio=1 parameter.  At least it
didn't 

Re: [PATCH 1/4] atl1: Build files for Attansic L1 driver

2007-01-11 Thread Christoph Hellwig
On Wed, Jan 10, 2007 at 06:40:51PM -0600, Jay Cliburn wrote:
 --- /dev/null
 +++ b/drivers/net/atl1/Makefile
 @@ -0,0 +1,30 @@
 +
 +#
 +# Attansic L1 gigabit ethernet driver
 +# Copyright(c) 2005 - 2006 Attansic Corporation.
 +#
 +# This program is free software; you can redistribute it and/or modify it
 +# under the terms and conditions of the GNU General Public License,
 +# version 2, as published by the Free Software Foundation.
 +#
 +# This program is distributed in the hope it will be useful, but WITHOUT
 +# ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
 +# FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
 +# more details.
 +#
 +# You should have received a copy of the GNU General Public License along 
 with
 +# this program; if not, write to the Free Software Foundation, Inc.,
 +# 51 Franklin St - Fifth Floor, Boston, MA 02110-1301 USA.
 +#
 +# The full GNU General Public License is included in this distribution in
 +# the file called COPYING.
 +#
 +

I don't think anyone can claim copyright on two lines of actual kbuild code.

 +#
 +# Makefile for the Attansic L1 gigabit ethernet driver
 +#

This comment is antirely superflous.

 +obj-$(CONFIG_ATL1) += atl1.o
 +
 +atl1-objs := atl1_main.o atl1_hw.o atl1_ethtool.o atl1_param.o

Thi should be atl1-y += ...

In short the whole contents of this file should be:

 snip 
obj-$(CONFIG_ATL1)  += atl1.o
atl1-y  += atl1_main.o atl1_hw.o atl1_ethtool.o atl1_param.o
 snip 

-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 2/4] atl1: Header files for Attansic L1 driver

2007-01-11 Thread Christoph Hellwig
On Wed, Jan 10, 2007 at 06:41:37PM -0600, Jay Cliburn wrote:
 +/**
 + * atl1.h - atl1 main header

Please remove these kind of comments, they get out of date far too soon
and don't really help anything.  (Also everywhere else in the driver)

 +#include linux/tcp.h
 +#include linux/skbuff.h
 +#include linux/netdevice.h
 +#include linux/pci.h
 +#include linux/spinlock_types.h
 +#include linux/workqueue.h
 +#include linux/timer.h
 +#include linux/delay.h
 +#include linux/if_vlan.h
 +
 +#include asm/types.h

Please always include linux/types.h


 +#include asm/atomic.h

Please only includ headers where you use them - that's mostly the .c
files unless you have lots of inlines or complex structures in the headers.



 +#ifdef NETIF_F_TSO
 +#include net/checksum.h
 +#endif
 +
 +#ifdef SIOCGMIIPHY
 +#include linux/mii.h
 +#endif
 +
 +#ifdef SIOCETHTOOL
 +#include linux/ethtool.h
 +#endif

Please remove all these ifdefs.

 +#define BAR_00

This looks etirely superflous.

 +#define usec_delay(x)udelay(x)
 +#ifndef msec_delay
 +#define msec_delay(x)do { if(in_interrupt()) { \
 + /* Don't mdelay in interrupt context!*/ \
 + BUG(); \
 + } else { \
 + msleep(x); \
 + }} while(0)
 +/**
 + * Some workarounds require millisecond delays and are run during interrupt
 + * context.  Most notably, when establishing link, the phy may need tweaking
 + * but cannot process phy register reads/writes faster than millisecond
 + * intervals...and we establish link due to a link status change interrupt.
 + **/
 +#define msec_delay_irq(x) mdelay(x)
 +#endif

Please kill all these wrappers.

 +} _ATL1_ATTRIB_PACK_;

All your structs seems to be properly packed from a first sight.  Please
doble-check this and get rid of the attribute packed.

 +struct csum_param {
 + unsigned buf_len:14;
 + unsigned dma_int:1;
 + unsigned pkt_int:1;
 + u16 valan_tag;
 + unsigned eop:1;
 + /* command */
 + unsigned coalese:1;
 + unsigned ins_vlag:1;
 + unsigned custom_chksum:1;
 + unsigned segment:1;
 + unsigned ip_chksum:1;
 + unsigned tcp_chksum:1;
 + unsigned udp_chksum:1;
 + /* packet state */
 + unsigned vlan_tagged:1;
 + unsigned eth_type:1;
 + unsigned iphl:4;
 + unsigned:2;
 + unsigned payload_offset:8;
 + unsigned xsum_offset:8;
 +} _ATL1_ATTRIB_PACK_;

Bitfields should not be used for hardware datastructures ever.
Please convert this to explicit masking and shifting.

 +/* Structure containing variables used by the shared code */
 +struct atl1_hw {
 + u8 __iomem *hw_addr;
 + void *back;

This is definitly a kernel data structure.  Shouldn't it be in atl1.h
instead of _hw.h?  Also does back really need to be a void pointer or
can it be something typed?

 + u16 dev_rev;
 + u16 device_id;
 + u16 vendor_id;
 + u16 subsystem_id;
 + u16 subsystem_vendor_id;
 + u8 revision_id;

Please just use the values from the pci_dev insead of duplicating them.

 +/* formerly ATL1_WRITE_REG */
 +static inline void atl1_write32(const struct atl1_hw *hw, int reg, u32 val)
 +{
 +writel(val, hw-hw_addr + reg);
 +}
 +
 +/* formerly ATL1_READ_REG */
 +static inline u32 atl1_read32(const struct atl1_hw *hw, int reg)
 +{
 +return readl(hw-hw_addr + reg);
 +}

Just kill all these wrappers.  Also you probably want to convert to
pci_iomap + ioread*/iowrite*.

-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 3/4] atl1: Main C file for Attansic L1 driver

2007-01-11 Thread Christoph Hellwig
 +#include asm/page.h

Why do you need this one?

 +#include asm/system.h

Shouldn't be needed aswell.

 +#include asm/checksum.h

net/checksum.h, please.

 + spin_lock_init(adapter-stats_lock);
 + spin_lock_init(adapter-tx_lock);
 + spin_lock_init(adapter-mb_lock);
 + spin_lock_init(adapter-vl_lock);
 + spin_lock_init(adapter-mii_lock);

Do you really need that many locks?

 +static inline void atl1_irq_enable(struct atl1_adapter *adapter)
 +{
 + if (likely(0 == atomic_dec_and_test(adapter-irq_sem)))
 + atl1_write32(adapter-hw, REG_IMR, IMR_NORMAL_MASK);
 +}

We normally prefer atomic_dec_and_test(adapter-irq_sem) == 0 or
just !atomic_dec_and_test(adapter-irq_sem).

Also all these little helpers should probably not be marked inline
so gcc can decide on it's own merit which static function to inline.

 +static int atl1_ioctl(struct net_device *netdev, struct ifreq *ifr, int cmd)
 +{
 + switch (cmd) {
 + case SIOCGMIIPHY:
 + case SIOCGMIIREG:
 + case SIOCSMIIREG:
 + return atl1_mii_ioctl(netdev, ifr, cmd);
 + case SIOCETHTOOL:
 + atl1_set_ethtool_ops(netdev);

ethtool doesn't use the ioctl path anymore at all.

 +static int atl1_open(struct net_device *netdev)
 +{
 + struct atl1_adapter *adapter = netdev_priv(netdev);
 + int err;
 +
 + /* allocate transmit descriptors */
 + if ((err = atl1_setup_ring_resources(adapter)))
 + return err;
 + if ((err = atl1_up(adapter)))
 + goto err_up;

Preffered style for this is:

err = atl1_setup_ring_resources(adapter);
if (err)
return err;
err = atl1_up(adapter);
if (err)
goto err_up;

(also applies in various other places)

-
To unsubscribe from this list: send 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 4/4] atl1: Ancillary C files for Attansic L1 driver

2007-01-11 Thread Christoph Hellwig
 +#define ATL1_STATS_LEN sizeof(atl1_gstrings_stats) / sizeof(struct 
 atl1_stats)

Just use an opencoded ARRAY_SIZE().

 +void atl1_read_pci_cfg(struct atl1_hw *hw, u32 reg, u16 * value)
 +{
 +struct atl1_adapter *adapter = hw-back;
 +pci_read_config_word(adapter-pdev, reg, value);
 +}
 +
 +void atl1_write_pci_cfg(struct atl1_hw *hw, u32 reg, u16 * value)
 +{
 +struct atl1_adapter *adapter = hw-back;
 +pci_write_config_word(adapter-pdev, reg, *value);
 +}

Please just kill these types of wrappers and use pci_read_config_word/
pci_write_config_word directly.

 +static inline bool atl1_eth_address_valid(u8 * p_addr)
 +{
 + /* Invalid PermanentAddress ? */
 + if (((p_addr[0] == 0) 
 +  (p_addr[1] == 0) 
 +  (p_addr[2] == 0) 
 +  (p_addr[3] == 0)  (p_addr[4] == 0)  (p_addr[5] == 0)
 + ) || (p_addr[0]  1)) 
 + /* Multicast address or Broadcast Address */
 + return false;
 +
 + return true;
 +}

Don't we have a generic helper for this kind of thing?

-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 1/4] atl1: Build files for Attansic L1 driver

2007-01-11 Thread Jeff Garzik
FWIW Jay is not the vendor, just someone who is helping clean up an ugly 
vendor driver.


Jeff



-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [IPv6] PROBLEM? Network unreachable despite correct route

2007-01-11 Thread Jarek Poplawski
On 10-01-2007 01:23, Bernhard Schmidt wrote:
 On Tue, Jan 09, 2007 at 08:36:24PM +0100, Bernhard Schmidt wrote:
...
 I'm having a really ugly problem I'm trying to pinpoint, but failed so
 far. I'm neither completely convinced it is not related to my local
 setup(s), nor do I have any clue how this might be caused.
...
 I managed to pull ip -6 route, ip -6 neigh and ip -6 addr while the box
 was not responding:
 
 ip -6 route:
 2001:4ca0:0:f000::/64 dev eth0  proto kernel  metric 256  expires 86322sec 
 mtu 1500 advmss 1440 fragtimeout 4294967295
 fe80::/64 dev eth0  metric 256  expires 21225804sec mtu 1500 advmss 1440 
 fragtimeout 4294967295
 ff00::/8 dev eth0  metric 256  expires 21225804sec mtu 1500 advmss 1440 
 fragtimeout 4294967295
 default via fe80::2d0:4ff:fe12:2400 dev eth0  proto kernel  metric 1024  
 expires 1717sec mtu 1500 advmss 1440 fragtimeout 64
 unreachable default dev lo  proto none  metric -1  error -101 fragtimeout 255

Did you analyze this dev lo warning? 

Regards,
Jarek P.
-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [IPv6] PROBLEM? Network unreachable despite correct route

2007-01-11 Thread Bernhard Schmidt

Jarek Poplawski wrote:


ip -6 route:
2001:4ca0:0:f000::/64 dev eth0  proto kernel  metric 256  expires 86322sec mtu 
1500 advmss 1440 fragtimeout 4294967295
fe80::/64 dev eth0  metric 256  expires 21225804sec mtu 1500 advmss 1440 
fragtimeout 4294967295
ff00::/8 dev eth0  metric 256  expires 21225804sec mtu 1500 advmss 1440 
fragtimeout 4294967295
default via fe80::2d0:4ff:fe12:2400 dev eth0  proto kernel  metric 1024  
expires 1717sec mtu 1500 advmss 1440 fragtimeout 64
unreachable default dev lo  proto none  metric -1  error -101 fragtimeout 255
Did you analyze this dev lo warning? 


That one is default. Recent kernels (since 2.6.12 or so, I think when 
the default on-link assumption was killed) have a default route pointing 
to unreachable default lo on bootup. Routes learned from RA or added 
statically are installed with a better metric and are preferred that way.


I think the use is to have a Network unreachable returned immediately if 
no IPv6 router is present.


Regards,
Bernhard
-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[2.6 patch] make hdlc_setup() static again

2007-01-11 Thread Adrian Bunk
hdlc_setup was exported, but this export was never used.

If a driver using it actually shows up it can still be exported again.

Signed-off-by: Adrian Bunk [EMAIL PROTECTED]
Acked-by: Krzysztof Halasa [EMAIL PROTECTED]

---

This patch was already sent on:
- 25 Nov 2006

--- linux-2.6.19-rc6-mm1/drivers/net/wan/hdlc.c.old 2006-11-25 
00:16:13.0 +0100
+++ linux-2.6.19-rc6-mm1/drivers/net/wan/hdlc.c 2006-11-25 00:16:26.0 
+0100
@@ -222,7 +222,7 @@
return -EINVAL;
 }
 
-void hdlc_setup(struct net_device *dev)
+static void hdlc_setup(struct net_device *dev)
 {
hdlc_device *hdlc = dev_to_hdlc(dev);
 
@@ -325,7 +325,6 @@
 EXPORT_SYMBOL(hdlc_open);
 EXPORT_SYMBOL(hdlc_close);
 EXPORT_SYMBOL(hdlc_ioctl);
-EXPORT_SYMBOL(hdlc_setup);
 EXPORT_SYMBOL(alloc_hdlcdev);
 EXPORT_SYMBOL(unregister_hdlc_device);
 EXPORT_SYMBOL(register_hdlc_protocol);

-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[2.6 patch] net/wanrouter/wanmain.c: cleanups

2007-01-11 Thread Adrian Bunk
This patch contains the following cleanups:
- make the following needlessly global functions static:
  - lock_adapter_irq()
  - unlock_adapter_irq()
- #if 0 the following unused global functions:
  - wanrouter_encapsulate()
  - wanrouter_type_trans()

Signed-off-by: Adrian Bunk [EMAIL PROTECTED]

---

This patch was already sent on:
- 12 Dec 2006
- 30 Jun 2006
- 5 Apr 2006

 include/linux/wanrouter.h |8 
 net/wanrouter/wanmain.c   |   17 -
 2 files changed, 8 insertions(+), 17 deletions(-)

--- linux-2.6.17-rc1-mm1-full/include/linux/wanrouter.h.old 2006-04-05 
17:03:07.0 +0200
+++ linux-2.6.17-rc1-mm1-full/include/linux/wanrouter.h 2006-04-05 
17:15:20.0 +0200
@@ -516,9 +516,6 @@
 /* Public functions available for device drivers */
 extern int register_wan_device(struct wan_device *wandev);
 extern int unregister_wan_device(char *name);
-__be16 wanrouter_type_trans(struct sk_buff *skb, struct net_device *dev);
-int wanrouter_encapsulate(struct sk_buff *skb, struct net_device *dev,
- unsigned short type);
 
 /* Proc interface functions. These must not be called by the drivers! */
 extern int wanrouter_proc_init(void);
@@ -527,11 +524,6 @@
 extern int wanrouter_proc_delete(struct wan_device *wandev);
 extern int wanrouter_ioctl( struct inode *inode, struct file *file, unsigned 
int cmd, unsigned long arg);
 
-extern void lock_adapter_irq(spinlock_t *lock, unsigned long *smp_flags);
-extern void unlock_adapter_irq(spinlock_t *lock, unsigned long *smp_flags);
-
-
-
 /* Public Data */
 /* list of registered devices */
 extern struct wan_device *wanrouter_router_devlist;
--- linux-2.6.17-rc1-mm1-full/net/wanrouter/wanmain.c.old   2006-04-05 
17:03:39.0 +0200
+++ linux-2.6.17-rc1-mm1-full/net/wanrouter/wanmain.c   2006-04-05 
17:18:32.0 +0200
@@ -144,8 +144,8 @@
 
 static struct wan_device *wanrouter_find_device(char *name);
 static int wanrouter_delete_interface(struct wan_device *wandev, char *name);
-void lock_adapter_irq(spinlock_t *lock, unsigned long *smp_flags);
-void unlock_adapter_irq(spinlock_t *lock, unsigned long *smp_flags);
+static void lock_adapter_irq(spinlock_t *lock, unsigned long *smp_flags);
+static void unlock_adapter_irq(spinlock_t *lock, unsigned long *smp_flags);
 
 
 
@@ -162,8 +162,8 @@
  * Organize Unique Identifiers for encapsulation/decapsulation
  */
 
-static unsigned char wanrouter_oui_ether[] = { 0x00, 0x00, 0x00 };
 #if 0
+static unsigned char wanrouter_oui_ether[] = { 0x00, 0x00, 0x00 };
 static unsigned char wanrouter_oui_802_2[] = { 0x00, 0x80, 0xC2 };
 #endif
 
@@ -304,6 +304,8 @@
return 0;
 }
 
+#if 0
+
 /*
  * Encapsulate packet.
  *
@@ -399,6 +401,7 @@
return ethertype;
 }
 
+#endif  /*  0  */
 
 /*
  * WAN device IOCTL.
@@ -860,23 +863,19 @@
return 0;
 }
 
-void lock_adapter_irq(spinlock_t *lock, unsigned long *smp_flags)
+static void lock_adapter_irq(spinlock_t *lock, unsigned long *smp_flags)
 {
spin_lock_irqsave(lock, *smp_flags);
 }
 
 
-void unlock_adapter_irq(spinlock_t *lock, unsigned long *smp_flags)
+static void unlock_adapter_irq(spinlock_t *lock, unsigned long *smp_flags)
 {
spin_unlock_irqrestore(lock, *smp_flags);
 }
 
 EXPORT_SYMBOL(register_wan_device);
 EXPORT_SYMBOL(unregister_wan_device);
-EXPORT_SYMBOL(wanrouter_encapsulate);
-EXPORT_SYMBOL(wanrouter_type_trans);
-EXPORT_SYMBOL(lock_adapter_irq);
-EXPORT_SYMBOL(unlock_adapter_irq);
 
 MODULE_LICENSE(GPL);
 


-
To unsubscribe from this list: send 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: [2.6 patch] net/wanrouter/wanmain.c: cleanups

2007-01-11 Thread Alexey Dobriyan

On 1/11/07, Adrian Bunk [EMAIL PROTECTED] wrote:

This patch contains the following cleanups:
- make the following needlessly global functions static:
  - lock_adapter_irq()
  - unlock_adapter_irq()


These are 1:1 wrappers around spinlock_irqsave/restore. Just remove them.
-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 2/4] atl1: Header files for Attansic L1 driver

2007-01-11 Thread Jay Cliburn
On Thu, Jan 11, 2007 at 09:27:04AM +, Christoph Hellwig wrote:
 On Wed, Jan 10, 2007 at 06:41:37PM -0600, Jay Cliburn wrote:
  +/**
  + * atl1.h - atl1 main header
 
 Please remove these kind of comments, they get out of date far too soon
 and don't really help anything.  (Also everywhere else in the driver)

Is your concern here with the filename portion of the comment only, or
with the entire comment including the copyright and other material?

Jay
-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 2/4] atl1: Header files for Attansic L1 driver

2007-01-11 Thread Stephen Hemminger
On Thu, 11 Jan 2007 10:53:30 -0600
Jay Cliburn [EMAIL PROTECTED] wrote:

 On Thu, Jan 11, 2007 at 09:27:04AM +, Christoph Hellwig wrote:
  On Wed, Jan 10, 2007 at 06:41:37PM -0600, Jay Cliburn wrote:
   +/**
   + * atl1.h - atl1 main header
  
  Please remove these kind of comments, they get out of date far too soon
  and don't really help anything.  (Also everywhere else in the driver)
 
 Is your concern here with the filename portion of the comment only, or
 with the entire comment including the copyright and other material?
 
 Jay

Comments should not insult the reader by stating the obvious.
-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 2/4] atl1: Header files for Attansic L1 driver

2007-01-11 Thread Christoph Hellwig
On Thu, Jan 11, 2007 at 10:53:30AM -0600, Jay Cliburn wrote:
 On Thu, Jan 11, 2007 at 09:27:04AM +, Christoph Hellwig wrote:
  On Wed, Jan 10, 2007 at 06:41:37PM -0600, Jay Cliburn wrote:
   +/**
   + * atl1.h - atl1 main header
  
  Please remove these kind of comments, they get out of date far too soon
  and don't really help anything.  (Also everywhere else in the driver)
 
 Is your concern here with the filename portion of the comment only, or
 with the entire comment including the copyright and other material?

Only the line I quoted.  Copyright statements and similar things are
obviously fine :)
-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html