Re: BUG: soft lockup detected on CPU#0! (2.6.18.2 plus hacks)
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)
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)
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
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!
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)
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
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
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
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
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
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.
[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
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.
[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
[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
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
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)
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?
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
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?
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
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
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
+#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
+#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
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
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
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
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
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
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
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
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
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